Test only with JRuby 9.2.21.0 in CI (disable 9.2.8.0) #2572
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**What does this PR do?**: This PR disables testing of JRuby 9.2.8.0 in CI, as well as appraisal generation for it (as well as removes the existing appraisals). **Motivation**: Maintaining JRuby 9.2.8.0 working has been quite a burden: this version has many, many, many known issues -- you will notice that there have been **13** other bugfix releases on top of it; latest JRuby is 9.2.31.0. Some examples of PRs where this JRuby version caused toil are #2565, #2558, #2448, #2412, #1869, etc We've decided to stop running it in CI for now, and plan to evaluate if we should remove it entirely from our test harness. We recommend that all our customers use JRuby 9.2.21.0, and also recommend evaluating the move to JRuby 9.3, as the JRuby developers have marked JRuby 9.2 as end-of-life: <https://twitter.com/jruby/status/1611063274459566106>. **Additional Notes**: I didn't have to remove the appraisal gemfiles from the tree, but since I disabled the tasks that automatically keep them up-to-date, they wouldn't be maintained anyway. If/when we do want to revert this change, we should regenerate them using the usual rake tasks. **How to test the change?**: Validate that CI is green and that the JRuby 9.2.8.0 configuration is not shown there.
lloeki
reviewed
Jan 20, 2023
| @@ -166,11 +166,11 @@ TRACER_VERSIONS = %w[ | |||
| 3.0 | |||
| 3.1 | |||
| 3.2 | |||
| jruby-9.2.8.0 | |||
There was a problem hiding this comment.
Maybe we should convert this %w[ to an actual array, so that we can add the comment inline, e.g:
3.1
3.2
#jruby-9.2.8.0 # TODO: jruby-9.2.8.0 disabled for possible removal
jruby-9.3
jruby-9.4
Minor sidenote: I'm not quite a fan of this Rubocop rule of forcing people to use %w[] % the likes. As an old Perl user I get where it comes from and there are definitely legit uses where it really adds to readability, but I feel like most of the time it doesn't really add to readability, or forces inconsistency and breaks rhythm which hampers readability.
The above is an example of that, where the jruby-9.2.8.0 specific comment gets to be hoisted out because of Rubocop.
There was a problem hiding this comment.
Yeah, I really dislike this Rubocop rule as well. I can never remember what's the correct one to use...
Fixed in 25534a2 .
There was a problem hiding this comment.
Thanks for the feedback about the %w[] y'all. I've jotted a note to open a separate PR to tweak our Rubocop rules about this :)
lloeki
approved these changes
Jan 20, 2023
ivoanjo
added a commit
that referenced
this pull request
Jan 20, 2023
**What does this PR do?**: The `Style/WordArray` cop (<https://www.rubydoc.info/github/bbatsov/RuboCop/RuboCop/Cop/Style/WordArray>) by default enforces this: ```ruby # good %w[foo bar baz] # bad ['foo', 'bar', 'baz'] ``` In #2572 both me, @lloeki and @TonyCTHsu agreed that we didn't particularly like the `%w[]` style, so I'm opening a PR to disable it. **Motivation**: Rubocop has a lot of extremely opinionated cops that even the Rubocop authors disable (which is why there are as many Rubocop config variants as there are Ruby projects...), so we shouldn't get too stuck on the defaults. **Additional Notes**: This cop could be configured in a flipped way (e.g. consider `%w[]` bad and `['foo']` good but I don't think it's worth enabling it. I can live with either, and we can change our minds at any time. **How to test the change?**: Write the example above to a file and confirm that Rubocop does not complain about the use of either style.
ivoanjo
added a commit
that referenced
this pull request
Mar 10, 2023
**What does this PR do?**: In #2572 we changed our CI setup to test only with JRuby 9.2.21.0, disabling JRuby 9.2.8.0. Prior to that change, we tested with both versions. At the time we decided to only disable the JRuby 9.2.8.0 testing, commenting it out. This PR actually removes all references to testing with JRuby 9.2.8.0. As a reminder, we recommend that all our customers use at least JRuby 9.2.21.0, and also recommend evaluating the move to JRuby 9.3, as the JRuby developers have marked JRuby 9.2 as end-of-life: <https://twitter.com/jruby/status/1611063274459566106>. **Motivation**: Avoid having leftover setup that's not in use. **Additional Notes**: N/A **How to test the change?**: Validate that CI is still green on all configurations.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?:
This PR disables testing of JRuby 9.2.8.0 in CI, as well as appraisal generation for it (as well as removes the existing appraisals).
Motivation:
Maintaining JRuby 9.2.8.0 working has been quite a burden: this version has many, many, many known issues -- you will notice that there have been 13 other bugfix releases on top of it; latest JRuby is 9.2.21.0.
Some examples of PRs where this JRuby version caused toil are
#2565, #2558, #2448, #2412, #1869, etc
We've decided to stop running it in CI for now, and plan to evaluate if we should remove it entirely from our test harness.
We recommend that all our customers use JRuby 9.2.21.0, and also recommend evaluating the move to JRuby 9.3, as the JRuby developers have marked JRuby 9.2 as end-of-life:
https://twitter.com/jruby/status/1611063274459566106.
Additional Notes:
I didn't have to remove the appraisal gemfiles from the tree, but since I disabled the tasks that automatically keep them up-to-date, they wouldn't be maintained anyway.
If/when we do want to revert this change, we should regenerate them using the usual rake tasks.
How to test the change?:
Validate that CI is green and that the JRuby 9.2.8.0 configuration is not shown there.