Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch download-test_readline to ruby script #127

Merged
merged 2 commits into from
Feb 22, 2020

Conversation

MSP-Greg
Copy link
Contributor

Instead of using platform specific scripts to download readline test files from ruby/ruby, use a Ruby script.

Didn't go crazy with error checking, as GitHub connecting to GitHub should be reliable. It does fail when a file does not exist (response.message != 'OK').

More changes are coming to using Ruby on Actions, and a cross platform script will be helpful.

@MSP-Greg
Copy link
Contributor Author

MSP-Greg commented Feb 17, 2020

When removing appveyor.yml files, it may fail, as AppVeyor can be configured to run default scripts if no yml file is present.

If one checks the AppVeyor settings page for the project, the 'left side' menu's bottom option is 'Delete project'. That's the easy way.

Otherwise, the menu options 'Build', 'Test', & 'Deployment' items all have options that should be set to 'Off'. I think doing that will not run any CI, but I'm not sure...

Copy link
Member

@nobu nobu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a good idea to manage the list in one place.

Copy link
Member

@aycabta aycabta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing. I think "Delete AppVeyor & Travis" should be separated. Would you split the commit to another Pull Request?

@MSP-Greg MSP-Greg force-pushed the download-test-readline-rb branch from cda3f9b to 7d867ea Compare February 19, 2020 00:46
@MSP-Greg
Copy link
Contributor Author

Thank you for fixing. I think "Delete AppVeyor & Travis" should be separated. Would you split the commit to another Pull Request?

Thank you. Done, see PR #128.

@MSP-Greg MSP-Greg force-pushed the download-test-readline-rb branch from 7d867ea to dff2855 Compare February 21, 2020 20:24
@MSP-Greg MSP-Greg force-pushed the download-test-readline-rb branch from dff2855 to 73d2e3c Compare February 22, 2020 01:29
@aycabta aycabta merged commit 97ac70e into ruby:master Feb 22, 2020
@MSP-Greg
Copy link
Contributor Author

@aycabta

Thanks. I realized last night while AFK that the code may not be thread safe with the handling of the file info array/matrix. If so, I'll do a PR...

@MSP-Greg MSP-Greg deleted the download-test-readline-rb branch February 22, 2020 13:44
@aycabta
Copy link
Member

aycabta commented Feb 22, 2020

All the discussions ware great. And thank you so much for your careful consideration. Your way what you did covers where I couldn't find problems.

the code may not be thread safe with the handling of the file info array/matrix.

You're always welcome!

@MSP-Greg
Copy link
Contributor Author

Thanks, I'm glad I could help. Thank you for reline, as it's a big improvement for Windows Ruby.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants