-
Notifications
You must be signed in to change notification settings - Fork 131
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
Support Rails 8.0 #258
base: master
Are you sure you want to change the base?
Support Rails 8.0 #258
Conversation
|
||
gem.add_dependency "minitest", "~> 5.20" | ||
gem.add_dependency "railties", ">= 7.2.0", "< 8.0.0" | ||
gem.add_dependency "railties", ">= 8.0.0", "< 8.1.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.
Please don't, see: svenfuchs/rails-i18n#1130 (comment)
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 minitest-rails is in a different situation than rails-i18n, since it aims to release with the same major and minor version as the Rails version it supports.
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.
Just an observation, because the commit doesn't say why. But the most recent activity like this chose to use < [next major version].0.0
as the upper bound.
a3b0780#diff-b0f4e53f9bc79026fa5f7eb8b63bdf2c87232f0ba7b91918d5896531e65fa980R21
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.
@blowmage What would you prefer here? < 8.1.0
, < 9.0.0
or no upper limit at all?
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.
@pdobb the comment right below that one does explain why: "This upper dependency require to do the same dance every few years [list of PRs] That prevent people from trying Rails release candidates etc. It's just a bad pattern."
An upper limit causes needless delay and churn when things don't actually need a change. Let's assume we remove the upper constraint and Rails 9 comes out as an example:
- If nothing breaks with minitest-rails: great! No PR/gem release needed to unblock apps to upgrade to the newest version of Rails. This is an improvement over the current way.
- If something breaks with minitest-rails: we'll wait for a new release to fix it. This is the same as today.
In my app's Gemfile I can pin specific versions if I know that works better for my situation, but I can't override the upper constraint as easily.
@@ -18,11 +18,10 @@ jobs: | |||
fail-fast: false | |||
matrix: | |||
ruby: | |||
- '3.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.
Ruby 3.1 EOL is 2025-03-31
See: https://www.ruby-lang.org/en/downloads/branches/
def describe(*, &block) | ||
cls = describe_before_minitest_spec_constant_fix(*, &block) |
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.
RuboCop recommends these changes now that we’re on Ruby 3.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.
Not sure this is a good idea because it won't work with JRuby
Rails 8.0.0 was released a few days ago.
Fixes #257