-
-
Notifications
You must be signed in to change notification settings - Fork 92
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 full Ruby 3.4 compatibility (non-breaking) #537
Add full Ruby 3.4 compatibility (non-breaking) #537
Conversation
Warning Rate limit exceeded@tagliala has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 37 minutes and 28 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe pull request introduces updates to GitHub Actions workflows and a Ruby helper method to support Ruby 3.4. The changes primarily involve expanding the testing matrix to include Ruby 3.4 in both the generator and Ruby workflows, with specific exclusion rules for certain Rails gemfiles. Additionally, a modification was made to the error message generation in the Shakapacker helper to address a potential issue with method caller identification in Ruby 3.4. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
# TODO: remove when Ruby < 3.1 support will be dropped | ||
# Ref: https://github.com/shakacode/shakapacker/issues/534 | ||
bundler_update_command = | ||
if RUBY_VERSION.start_with?("3.1") | ||
"gem update bundler" | ||
else | ||
"gem install bundler --version '< 2.6'" | ||
end | ||
|
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.
@tagliala Can you shine some more light on this? Why we use override for anything that's not 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.
I've provided info here: #534
The failure is visible here: https://github.com/shakacode/shakapacker/actions/runs/12455226553/job/34767380887#step:4:140
Bundler 2.6 requires Ruby 3.1+, but for some reasons when you run gem update bundler
on 3.0 it prefers to fail rather than installing 2.5 series
I think this is slightly related to ruby/setup-ruby@540484a
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.
Thanks @tagliala I was wondering more why we only want it for 3.1 and not 3.2, 3.3 etc
Regardless, see https://github.com/shakacode/shakapacker/pull/539/files I think that's more sensible way to go about it as I don't see particularly good reason to update bundler manually.
Could you roll those changes here, together with any other workflow updates spanning this PR (like new Node version) and #536 and I'll happily merge
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.
Oh, #539 is much better, thanks. If you can merge I'll rebase
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.
Sounds good, stuck waiting for review on that one to merge but hopefully coming soon!
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.
@tagliala good to rebase!
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.
Rebased, comment resolved
.github/workflows/dummy.yml
Outdated
@@ -24,7 +24,7 @@ jobs: | |||
node-version: '16' | |||
- uses: ruby/setup-ruby@v1 | |||
with: | |||
ruby-version: '3.1.2' | |||
ruby-version: '3.1.6' |
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 would update this to 3.4 or at very least 3.3. Can't see decent reason to test with old Ruby version. Same applies to node 16 above. Let's use Node 22
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.
Since we're using 3.4 in ruby.yml let's stick to that!
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.
Sorry, I've reverted the change here because the dummy app uses Rails 6.1, so it should use 3.1.x
Since this commit does not belong to this feature, can be tracked separately
0bf68fa
to
12ba7c3
Compare
12ba7c3
to
70f043b
Compare
70f043b
to
fd7023f
Compare
Caller location with `label` is being decorated with `Shakapacker::Helper#` in Ruby 3.4 This commit changes `label` to `base_label` Close shakacode#535 Ref: - https://ruby-doc.org/core-2.7.0/Thread/Backtrace/Location.html#method-i-base_label
fd7023f
to
ee710e1
Compare
Blocked by
Caller location with
label
is being decorated withShakapacker::Helper#
in Ruby 3.4This commit changes
label
tobase_label
Close #535
Ref:
Summary by CodeRabbit
Release Notes
CI/CD Updates
Code Improvements