-
-
Notifications
You must be signed in to change notification settings - Fork 327
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
Add ruby 3.1 & drop ruby 2.4 #529
Conversation
Rubocop 1.22.1 is the first version that doesn't fail with the following error on ruby 3.1: ``` An error occurred while Layout/BlockAlignment cop was inspecting /home/runner/work/cucumber-rails/cucumber-rails/features/support/aruba.rb:3:0. ``` This version of rubocop doesn't support ruby 2.4.
5d437b1
to
ce780d3
Compare
- { ruby: '2.5', gemfile: 'rails_6_1' } | ||
- { ruby: '2.5', gemfile: 'rails_7_0' } | ||
# 3.0 -> Rails 6.1 and 7.0 | ||
# 3.1 -> Only 7.0 |
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.
comments appears missplaced
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.
Can you elaborate or make a suggestion? I see them fine on the final file (tabs alignment and above the first exclusion item).
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.
I think given the way the ruby codebase is going this makes a lot of sense. I do envisage us dropping 2.5 usage from this gem in the near future.
I would probably say we'll keep it on longer just because we do anticipate a lot more users for this to use lower ruby versions
cucumber-rails.gemspec
Outdated
@@ -2,7 +2,7 @@ | |||
|
|||
$LOAD_PATH.unshift File.expand_path('lib', __dir__) | |||
|
|||
Gem::Specification.new do |s| | |||
Gem::Specification.new do |s| # rubocop:disable Gemspec/RequireMFA |
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.
can we avoid these and have a reason in the standard file.
Also this appears to be new. Should we maybe be doing this?
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.
Moved the disabling to rubocop.yml
.
I'm not familiar with this setting. I'm not sure what changes are required on the release process and, sadly, I don't have time to investigate.
I disabled the rule to defer the decision.
My ideas:
- Defere the introduction of this setting by downgrading rubocop to 1.22.1. RequreMFA was introduced in 1.23
- Disable RequireMFA rule and make an issue saying that we should deal with it (defere decision but have a reminder)
- Enable RequireMFA and deal with it on next gem release
I choose number 2 but I can change it. Also open to other suggestions
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 deprecate ruby versions along with rails versions we choose to deprecate?
I wonder if many people runs rails 5.2 with ruby 2.7 ?
Equivalent ruby/rails version, IMO
Rails 4.2 - 2.5 works great, 2.6 needs a hack to run the tests but works great. (1.9.3 is still minimum supported)
Rails 5.x - 2.2.2 becomes minimum, 2.5/2.6 works great obviously.
Rails 6.0 - 2.5 is new minimum, we should be moving to 2.7 soonish
Rails 6.1 - 2.5 remain the minimum, 3.0 is legit candidate
Rails 7 - now requires Ruby 2.7
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.
We do this with our running matrix.
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.
Great work thanks. Also thanks for putting the links to the MFA thing.
On first glance I think we won't want it, but not 100%
Before merging can you update changelog and readme with refs to ruby 3.1 |
@luke-hill updated CHANGELOG. I don't know where to put a ref on the README. I'm happy to do it if you point me in the right direction. |
Evidently nowhere haha. I'll merge this in AS IS. |
Summary
Details
Rubocop requires an upgrade to 1.22.1 or higher to run on Ruby 3.1. Older versions generate the following error
Rubocop 1.22.1 doesn't support ruby 2.4. Since ruby 2.4 EOL date is 2020-03-31 (almost two years ago) dropping support to add the latest ruby seems a good call.
The PR also downgrades ruby 2.5 version from 2.5.9 to 2.5.8. The reason is the following. Upgrading rubocop introduced a new dependency
unicode-display_width
. Bundler is unable to download it dueArgumentError: wrong number of arguments (given 4, expected 1)
. According to this comment the problem is that we're using an old version of rubygems that does not support psych 4.Ruby installation and bundle install is done by ruby/setup-ruby in a single step. I couldn't find a way to upgrade rubygems before running bundle install without extending the action .
Ruby 2.5.8 doesn't have this problem. Considering that ruby 2.5 EOL date is 2021-03-31 (almost a year ago), I believe it's fine to run tests against 2.5.8
Motivation and Context
Add support to latest ruby version
Types of changes