-
-
Notifications
You must be signed in to change notification settings - Fork 109
< 8000 bdi class="js-issue-title markdown-title">Revert "Drop MRI 1.8.7-2.2 support (#158)" #250
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
base: main
Are you sure you want to change the base?
Conversation
end | ||
end | ||
ERROR | ||
exit(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rails/Exit: Do not use exit in Rails applications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow Hound is a mess. 🤯
This reverts commit 92d5134.
bin/bundle
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to lint this file : it is generated and about to disappear rubygems/rubygems#8345
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 the complication is the linting is automated when running the default rake task now, and violations will clog up the new lint lockfile. This will encourage us to get rid of files we don't use!
run: | | ||
case ${RUBY_VERSION} in | ||
1.8|1.9|2.0|2.1|2.2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should add EOL Rubies in matrix:ruby
above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's my next task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding them one by one now...
@pboling thank you! 👍 |
- Support debug for debugging with Ruby >= 2.7 - Support byebug for debugging with Ruby < 2.7
…d matrix - all current jruby and truffleruby are compatible with Ruby 3.1 at minimum, thus are supported by latest bundler
- Use strip_heredoc uniformly in acceptance tests
- Differences in handling of whitespace between versions of Ruby are often seen as minor, and go unnoticed unless tests explicitly test white space as appraisal's test suite does.
@n-rodriguez @nickcharlton Getting the test suite working with Ruby 2.7 was a lot of work. The code had drifted away from compatibility with Ruby 2.7, since it wasn't in the test suite. I've only added Ruby 2.7 and 3.4 to the matrix here, and my next task will be to add each Ruby down the line in a discrete PR, while maintaining compatibility with the newer Rubies. The latest spec failures are all related to whitespace on empty lines within block elements of the generated appraisals, and I'm working on a solution for that so the various Rubies don't have to match there exactly. |
- Ruby < 2 compat
@n-rodriguez @nickcharlton I have independently verified that these changes result in compatibility with every MRI Ruby 1.9.3+, and JRuby 9.1+ in a gem I pinned to use a branch on my fork that integrates these changes with the Ruby 1.8.7 can't be run any more on GHA, so we can't test it... I don't think that's a major issue. The RSpec project is in the same situation. I was doing that so I could release a new version of that gem ( Once I do that the whole CI will be green. |
- Possibly due to bundler issue on MRI and JRuby engines - rubygems/rubygems#8518
@n-rodriguez @nickcharlton This is green as a forest. :) |
@nickcharlton ping! |
Ooh, fascinating. I'm only just starting to look at this. My initial impression (when I saw the notification weeks ago…) was that going back this far was a bit silly, but I'm impressed by how you've been able to actually make it work. Did you have any thoughts on running the really old versions of Ruby in the tests? Do Docker images still exist that could be used? |
I'd like to split this PR into a few chunks:
I'd like to get to one commit (and also one PR) per logical change so it's easier to track back to what commit did what. Would you mind doing that? I know it's a lot of work, so I'll make sure we can merge these in close to when you open them. You might need to stack them on many branches, so potentially it's worth doing it over a couple of days so we don't make things incredibly confusing. |
So, there are other projects that run tests against ancient Ruby, such as RSpec, which still tests against Ruby 1.8.7. They use a docker image for that. GitHub actions can build back to Ruby 1.9.3, which means getting that far back is a mostly simple path. I didn't add it all here, because the PR was already so huge. Regarding migrating smaller sets of Ruby compatibility, I have some thoughts.
I'm going to restate, with some adjustments:
I want to get the new CI in place structurally, so it can easily incorporate the older Rubies as I add them, so that's why the CI upgrade is the first PR in the list above. Does that sound good @nickcharlton ? I may subdivide further if additional clean breaks present themselves, with a focus on small PRs. I am making heavy use of my fork + branch that has all of this plus the |
This reverts commit 92d5134.
Also fixes a few bugs.
eval_gemfile
feature.Also fixes other CI consistency issues, and adds lint / style enforcement for Ruby 1.8+ via rubocop-lts.