Upgrade ruby/rails #242
Upgrade ruby/rails #242
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe updates modernize the project's Ruby and Rails compatibility by adjusting CI matrices, appraisal files, and gem dependencies for newer versions. Several new gemfiles for Rails 7.1, 7.2, and 8.0 were introduced, and test group gems were removed from some gemfiles. The gemspec was enhanced with new runtime and development dependencies. A deprecated Ruby method in tests was replaced. Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
File.exists? has been replaced by File.exist? since ruby 3.2.0
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
deface.gemspec (1)
42-42: Maintain alphabetical order to satisfy RuboCopRuboCop flags the new
drbdependency ordering. Move it beforepolyglot:spec.add_dependency('rainbow', '>= 2.1.0') -spec.add_dependency('polyglot') spec.add_dependency('drb') +spec.add_dependency('polyglot')gemfiles/rails_7.1.gemfile (2)
5-8: Alphabetise gems to silence Bundler/OrderedGems-gem "concurrent-ruby" -gem "mutex_m" -gem "actionview", "~> 7.1.0" +gem "actionview", "~> 7.1.0" +gem "concurrent-ruby" +gem "mutex_m"
10-14: Keep test-group gems alphabetised as well- gem "test-unit" gem "pry" gem "pry-byebug" + gem "test-unit"gemfiles/rails_8.0.gemfile (2)
5-8: Enforce alphabetical ordering of top-level gems
RuboCop flags the order—actionviewandrailtiesshould precedeconcurrent-rubyandmutex_m.- gem "concurrent-ruby" - gem "mutex_m" - gem "actionview", "~> 8.0.0" - gem "railties", "~> 8.0.0" + gem "actionview", "~> 8.0.0" + gem "concurrent-ruby" + gem "mutex_m" + gem "railties", "~> 8.0.0"
10-14: Alphabetize gems in the:testgroup
RuboCop suggestspryandpry-byebugcome beforetest-unit.- group :test do - gem "test-unit" - gem "pry" - gem "pry-byebug" - end + group :test do + gem "pry" + gem "pry-byebug" + gem "test-unit" + endgemfiles/rails_7.2.gemfile (2)
5-8: Enforce alphabetical ordering of top-level gems
RuboCop flags thatactionviewandrailtiesshould precedeconcurrent-rubyandmutex_m.- gem "concurrent-ruby" - gem "mutex_m" - gem "actionview", "~> 7.2.0" - gem "railties", "~> 7.2.0" + gem "actionview", "~> 7.2.0" + gem "concurrent-ruby" + gem "mutex_m" + gem "railties", "~> 7.2.0"
10-14: Alphabetize gems in the:testgroup
To satisfy Bundler’s ordered-gems rule,pryandpry-byebugshould come beforetest-unit.- group :test do - gem "test-unit" - gem "pry" - gem "pry-byebug" - end + group :test do + gem "pry" + gem "pry-byebug" + gem "test-unit" + end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/main.yml(1 hunks)Appraisals(1 hunks)Gemfile(1 hunks)deface.gemspec(2 hunks)gemfiles/rails_7.0.gemfile(1 hunks)gemfiles/rails_7.1.gemfile(1 hunks)gemfiles/rails_7.2.gemfile(1 hunks)gemfiles/rails_8.0.gemfile(1 hunks)spec/deface/precompiler_spec.rb(1 hunks)
🧰 Additional context used
🪛 RuboCop (1.75.5)
deface.gemspec
[convention] 42-42: Dependencies should be sorted in an alphabetical order within their section of the gemspec. Dependency drb should appear before polyglot.
(Gemspec/OrderedDependencies)
[warning] 54-54: Do not use RUBY_VERSION in gemspec file.
(Gemspec/RubyVersionGlobalsUsage)
gemfiles/rails_7.1.gemfile
[convention] 7-7: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem actionview should appear before mutex_m.
(Bundler/OrderedGems)
[convention] 12-12: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem pry should appear before test-unit.
(Bundler/OrderedGems)
gemfiles/rails_7.2.gemfile
[convention] 7-7: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem actionview should appear before mutex_m.
(Bundler/OrderedGems)
[convention] 12-12: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem pry should appear before test-unit.
(Bundler/OrderedGems)
gemfiles/rails_8.0.gemfile
[convention] 7-7: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem actionview should appear before mutex_m.
(Bundler/OrderedGems)
[convention] 12-12: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem pry should appear before test-unit.
(Bundler/OrderedGems)
🔇 Additional comments (9)
Gemfile (1)
3-4: Validate necessity of new top-level runtime dependenciesAdding
concurrent-rubyandmutex_mas unconditional runtime gems increases every consumer’s install set.
Please double-check that Deface itself (not just the test suite or a forthcoming feature branch) really requires these libraries at runtime; if they are only needed for a specific Rails version or an optional feature, consider moving them under an appropriategroupor declaring them as soft dependencies in the gemspec.gemfiles/rails_7.0.gemfile (1)
5-6: Confirm that extra gems are truly required for Rails 7.0Same concern as in the main Gemfile – if
concurrent-ruby/mutex_mare not mandatory for Deface on Rails 7.0, the extra install cost can be avoided by scoping them (e.g. via appraisal-only groups).spec/deface/precompiler_spec.rb (1)
34-34: Good catch replacing deprecatedFile.exists?The switch to
File.exist?removes Ruby 3.x deprecation warnings – looks correct..github/workflows/main.yml (2)
17-20: Include Rails 8.0 combos in CI matrix
The matrix now tests Ruby 3.4 and 3.3 with Rails 8.0, aligning with the newrails_8.0.gemfile.
24-27: Include Rails 7.2 combos in CI matrix
Ruby 3.4 and 3.3 are now paired with Rails 7.2, matching the introducedrails_7.2.gemfile.Appraisals (4)
6-6: Update Rails 7.0 appraisal version constraint
The appraisal forrails-7.0is now locked to~> 7.0.0, matching the new Rails support matrix.
10-15: Introduce Rails 7.1 appraisal
Addedappraise 'rails-7.1'with~> 7.1.0constraint to cover the new gemfile.
19-24: Introduce Rails 7.2 appraisal
Addedappraise 'rails-7.2'scoped to~> 7.2.0, consistent with the CI and gemfile.
28-33: Introduce Rails 8.0 appraisal
Addedappraise 'rails-8.0'with~> 8.0.0, covering the latest Rails variant.
| - ruby: "3.3" | ||
| gemfile: "rails_7.1" |
There was a problem hiding this comment.
Missing gemfile for Rails 7.1 CI combo
You’ve added a CI job for gemfile: "rails_7.1", but there’s no corresponding gemfiles/rails_7.1.gemfile in the changes. Please ensure that file is present or remove the combo to avoid CI failures.
🤖 Prompt for AI Agents
In .github/workflows/main.yml at lines 29 to 30, you added a CI job for gemfile
"rails_7.1" but did not include the corresponding gemfiles/rails_7.1.gemfile
file. To fix this, either add the missing rails_7.1.gemfile to the gemfiles
directory with the appropriate dependencies or remove the rails_7.1 gemfile
entry from the CI workflow to prevent failures.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
deface.gemspec (2)
38-44: Sort runtime dependencies alphabetically.RuboCop’s
Gemspec/OrderedDependenciesflagged that the runtime dependencies aren’t in alphabetical order. Reorder thesespec.add_dependencylines within the runtime section:-spec.add_dependency('base64') -spec.add_dependency('bigdecimal') -spec.add_dependency('mutex_m') -spec.add_dependency('nokogiri', '>= 1.6') -spec.add_dependency('rainbow', '>= 2.1.0') -spec.add_dependency('polyglot') -spec.add_dependency('drb') +spec.add_dependency('base64') +spec.add_dependency('bigdecimal') +spec.add_dependency('drb') +spec.add_dependency('mutex_m') +spec.add_dependency('nokogiri', '>= 1.6') +spec.add_dependency('polyglot') +spec.add_dependency('rainbow', '>= 2.1.0')
55-56: Alphabetize development dependencies.RuboCop flagged that
pry-byebugshould come beforetest-unit. Swap these lines to maintain alphabetical order:-spec.add_development_dependency('test-unit') -spec.add_development_dependency('pry-byebug') +spec.add_development_dependency('pry-byebug') +spec.add_development_dependency('test-unit')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Gemfile(0 hunks)deface.gemspec(2 hunks)gemfiles/rails_7.0.gemfile(0 hunks)gemfiles/rails_7.1.gemfile(1 hunks)gemfiles/rails_7.2.gemfile(1 hunks)gemfiles/rails_8.0.gemfile(1 hunks)
💤 Files with no reviewable changes (2)
- gemfiles/rails_7.0.gemfile
- Gemfile
✅ Files skipped from review due to trivial changes (1)
- gemfiles/rails_8.0.gemfile
🚧 Files skipped from review as they are similar to previous changes (2)
- gemfiles/rails_7.1.gemfile
- gemfiles/rails_7.2.gemfile
🧰 Additional context used
🪛 RuboCop (1.75.5)
deface.gemspec
[convention] 43-43: Dependencies should be sorted in an alphabetical order within their section of the gemspec. Dependency polyglot should appear before rainbow.
(Gemspec/OrderedDependencies)
[convention] 44-44: Dependencies should be sorted in an alphabetical order within their section of the gemspec. Dependency drb should appear before polyglot.
(Gemspec/OrderedDependencies)
[convention] 56-56: Dependencies should be sorted in an alphabetical order within their section of the gemspec. Dependency pry-byebug should appear before test-unit.
(Gemspec/OrderedDependencies)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
deface.gemspec (1)
51-57: Reorder development dependencies
Alphabetical ordering is prescribed by RuboCop:prymust come beforepry-byebug.-spec.add_development_dependency('pry-byebug') -spec.add_development_dependency('pry') +spec.add_development_dependency('pry') +spec.add_development_dependency('pry-byebug')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Gemfile(0 hunks)deface.gemspec(1 hunks)gemfiles/rails_7.0.gemfile(0 hunks)gemfiles/rails_7.1.gemfile(1 hunks)gemfiles/rails_7.2.gemfile(1 hunks)gemfiles/rails_8.0.gemfile(1 hunks)
💤 Files with no reviewable changes (2)
- Gemfile
- gemfiles/rails_7.0.gemfile
🚧 Files skipped from review as they are similar to previous changes (3)
- gemfiles/rails_8.0.gemfile
- gemfiles/rails_7.2.gemfile
- gemfiles/rails_7.1.gemfile
🧰 Additional context used
🪛 RuboCop (1.75.5)
deface.gemspec
[convention] 53-53: Dependencies should be sorted in an alphabetical order within their section of the gemspec. Dependency pry should appear before pry-byebug.
(Gemspec/OrderedDependencies)
🔇 Additional comments (2)
deface.gemspec (2)
36-38: Update Rails dependencies for 7.0+
The loop correctly bumps bothactionviewandrailtiesto>= 7.0, aligning the gemspec with the new CI and appraisal configurations.
39-45: Introduce new runtime dependencies
The addition and ordering ofbase64,bigdecimal,drb,mutex_m,nokogiri,polyglot, andrainbowmatch expectations for required stdlib and external gems.
elia
left a comment
elia
left a comment
There was a problem hiding this comment.
Cutting off old versions will probably require a major release
2 participants
Summary by CodeRabbit