-
Notifications
You must be signed in to change notification settings - Fork 8
Ruby 3 Upgrade #517
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
Ruby 3 Upgrade #517
Conversation
b30f166 to
72d5b30
Compare
72d5b30 to
b93269a
Compare
|
All of our apps and most projects that are in active development have been migrated to Ruby 3.2.2. I think it's time for release-toolkit to make the transition too? |
b93269a to
b624f50
Compare
|
Great @crazytonyli, thanks for the heads up 👍 I've rebased the PR and will mark it as Ready for Review. |
| # Rule enforced after migrating to Ruby 3, to prevent the new syntax which allows for hash values and keyword arguments | ||
| # omission, such as `myMethod(x:, y:)` and `h = { a:, b: }`. | ||
| Style/HashSyntax: | ||
| EnforcedShorthandSyntax: never |
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.
May I know why adding this rule? I think it'd good to adopt Ruby 3 features from now on?
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.
@crazytonyli see paaHJt-5yi-p2
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! I have seen that post, but it didn't come to me while reviewing the code. 🙈
crazytonyli
left a 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.
Thanks for the changes! Probably worth P2 about it, considering there may be linting issues from rubocop while we write new code. 😄
| matrix: | ||
| setup: | ||
| ruby: | ||
| - 2.7.4 |
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 trunk branch protection rule needs to be changed to accommodate the pipeline change.
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, I've seen there's this required check that is pending, but was removed.
I don't seem to have access to the branch protection rules in release-toolkit though 🤔 . In fact, I don't see the Settings tab at all. Perhaps you or someone else can change it / change my role in the repo?
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.
Done!
What does it do?
Building on top of #492 and related PRs, this PR finally upgrades
release-toolkit's Ruby version to3.2.2, also fixing all reported Rubocop offences (see commits breakdown) and updates known issues (.rubocop_todo.yml).Note: This PR should be merged only when we're ready to support Ruby 3 on all client projects (or at least when we're sure they will jump straight to the
release-toolkitRuby 3 version).Checklist before requesting a review
bundle exec rubocopto test for code style violations and recommendationsspecs/*_spec.rb) if applicablebundle exec rspecto run the whole test suite and ensure all your tests passCHANGELOG.mdfile to describe your changes under the appropriate existing###subsection of the existing## Trunksection.MIGRATION.mdfile to describe how the changes will affect the migration from the previous major version and what the clients will need to change and consider.