Make root RuboCop bundle lint-only (no Rails/test stack)#3840
Make root RuboCop bundle lint-only (no Rails/test stack)#3840justin808 wants to merge 2 commits into
Conversation
Follow-up to #3836. PR #3836 centralized RuboCop ownership in the committed root Gemfile/Gemfile.lock but kept the root Gemfile eval_gemfile-ing react_on_rails/Gemfile, so a root `bundle install` still pulled in the full OSS Rails/dev/test/runtime stack instead of only lint gems. This makes the root Gemfile a pure lint/tooling bundle: - Remove `eval_gemfile File.expand_path("react_on_rails/Gemfile", ...)` and add an explicit `source "https://rubygems.org"`. - Declare only the gems RuboCop's config needs: rubocop, rubocop-performance, rubocop-rspec (matching the root .rubocop.yml `require:` plugins), pinned to the same versions the package locks resolve (1.61.0 / 1.20.2 / 2.31.0). - Add bare `rspec (~> 3.13)` so benchmark.yml's "Run benchmark script specs" step (`bundle exec rspec benchmarks/spec`, which #3836 wired to the root bundle) keeps working. Those specs deliberately load no Rails and no react_on_rails gem (see benchmarks/spec/spec_helper.rb), so bare rspec — not the package's rspec-rails stack — is all the root bundle needs. - Regenerate the committed root Gemfile.lock: it now resolves 25 lint/tooling gems (down from ~150+) with no rails, actionpack, activesupport, nokogiri, shakapacker, rspec-rails, runtime capybara, or react_on_rails PATH gem. PLATFORMS keeps ruby/arm64-darwin/x86_64-linux so the committed-lock platform check stays green. - Update AGENTS.md to describe the root Gemfile as a lint/tooling-only bundle. The package Gemfiles continue to own RBS/test/runtime deps. Root RuboCop commands still resolve from the one committed root lock; CI lint steps already point BUNDLE_GEMFILE at the root Gemfile (from #3836), so they are unaffected. Verification (local, Ruby 3.3.7, Bundler 4.0.10): - root `bundle install`: 25 gems, no rails/rspec-rails/actionpack - `cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop`: 217 files, 0 offenses - `cd react_on_rails_pro && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop --ignore-parent-exclusion`: 231 files, 0 offenses - root `bundle exec rubocop benchmarks`: 32 files, 0 offenses - root `bundle exec rspec benchmarks/spec`: 238 examples, 0 failures - script/check-gemfile-lock-platforms: all 6 locks include x86_64-linux Fixes #3837 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRoot ChangesRoot Gemfile Lint-Only Refactor
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly Related Issues
Possibly Related PRs
Suggested Labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Code Review: Make root RuboCop bundle lint-onlyOverall: Clean, well-reasoned change. Ready to merge. SummaryThis PR drops the Positives
Minor observations1. Version constraint style inconsistency — The three RuboCop gems use exact-version pins ( Consider tightening to 2. Version drift has no automated backstop The comment notes that the root's pinned versions must match the package lock's resolved versions, but there's no CI check to enforce this. If someone bumps rubocop in 3. Line 151 of run: bundle exec rspec benchmarks/specThis relies on the preceding 4. Platform list reduction — informational PLATFORMS dropped from 9 variants to 3 ( VerdictItems 1–4 are all minor polish or informational; none is blocking. The core change is correct and the tradeoff is clearly explained. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| gem "rubocop-performance", "1.20.2", require: false | ||
| gem "rubocop-rspec", "2.31.0", require: false | ||
|
|
||
| gem "rspec", "~> 3.13", require: false |
There was a problem hiding this comment.
The three RuboCop gems use exact-version pins (= x.y.z) but rspec uses a pessimistic constraint. The header comment says "pinned versions must match what the package locks resolve," which only holds for exact pins. A bundle update rspec in isolation could silently drift the root lock ahead of what the package Gemfiles resolve.
Consider pinning exactly to match the lock:
| gem "rspec", "~> 3.13", require: false | |
| gem "rspec", "3.13.2", require: false |
Or, if patch-level flexibility is intentional, add a note here clarifying that rspec is explicitly allowed to float within the minor unlike the RuboCop gems.
Greptile SummaryThis PR converts the root
Confidence Score: 5/5Safe to merge — the change is a straightforward bundle isolation with no runtime code affected. All three files change only Gemfile declarations and documentation. The root bundle now correctly declares only the four gems it actually needs. Removing native-gem platforms from the lockfile is safe because every remaining gem is pure Ruby, so the generic No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Root Gemfile] -->|BEFORE| B[eval_gemfile react_on_rails/Gemfile]
B --> C[Full Rails/test stack ~150+ gems]
C --> D[rubocop + plugins]
C --> E[rails, nokogiri, rspec-rails, ...]
A2[Root Gemfile] -->|AFTER| F[source rubygems.org only]
F --> G[rubocop 1.61.0]
F --> H[rubocop-performance 1.20.2]
F --> I[rubocop-rspec 2.31.0]
F --> J[rspec ~> 3.13 for benchmarks/spec]
G & H & I & J --> K[Root Gemfile.lock ~25 gems]
L[react_on_rails/Gemfile] --> M[Package Gemfile.lock: Rails/test/runtime stack]
style C fill:#f96,stroke:#333
style K fill:#6f9,stroke:#333
Reviews (1): Last reviewed commit: "Make root RuboCop bundle lint-only (no R..." | Re-trigger Greptile |
Their gemfiles and locks should not include rubocop or any of its extensions, and rspec should be declared in shared file all of them import. |
CI runs the root bundle on Ruby 4.0.5, where benchmark is no longer a default gem. rubocop 1.61.0's executable requires it, and the lint-only bundle no longer pulls it in transitively via the Rails stack, so the lint-js-and-ruby job failed with LoadError. Declare it explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.
Reviewed by Cursor Bugbot for commit 9b7c081. Configure here.
| # It deliberately does NOT eval_gemfile the react_on_rails package Gemfile, | ||
| # so a root `bundle install` for lint-only workflows stays fast and does not | ||
| # pull in the Rails/test/runtime stack. The package Gemfiles continue to own | ||
| # their RBS/test/runtime dependencies. |
There was a problem hiding this comment.
Root rake breaks lint bundle
Medium Severity
Stopping the root Gemfile from loading the package bundle leaves AGENTS.md’s root bundle plus rake / rake run_rspec:gem flow without Rails and other package gems. The monorepo root Rakefile still eager-loads react_on_rails/rakelib, which requires rails/version before tasks run.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9b7c081. Configure here.
Code Review: Make root RuboCop bundle lint-onlyOverall: Clean, well-motivated change. Approachable with one minor note. What this does well
Minor:
|
| gem "rubocop-performance", "1.20.2", require: false | ||
| gem "rubocop-rspec", "2.31.0", require: false | ||
|
|
||
| gem "rspec", "~> 3.13", require: false |
There was a problem hiding this comment.
The comment above (lines 17–19) says the pinned versions must match what the package locks resolve, but rspec uses ~> 3.13 while all three rubocop gems use exact pins. The current resolved version is 3.13.2 (visible in the new lock). A minor rspec bump wouldn't break the benchmark specs since they load no Rails, but it creates a subtle inconsistency with the stated intent. Either pin exactly:
| gem "rspec", "~> 3.13", require: false | |
| gem "rspec", "3.13.2", require: false |
…or add a brief comment explaining why rspec intentionally gets a range constraint instead of an exact pin (e.g. "benchmarks/spec is stdlib-only so any 3.13.x is safe").
Pro Node Renderer Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
Core Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
|
Closing as superseded by #3841. This PR narrowed the root Gemfile but left the root Rakefile eager-loading package tasks under the slim root bundle, which Cursor correctly flagged as breaking root rake flows. #3841 carries the same #3837 goal plus the Rakefile delegation/setup updates needed to keep root commands working with separate root/package bundles. |
Pro (shard 1/2) Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
Pro (shard 2/2) Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
2 participants
Summary
Follow-up to #3836. That PR centralized RuboCop ownership in the committed root
Gemfile/Gemfile.lock, but it kept the rootGemfiledoingeval_gemfile File.expand_path("react_on_rails/Gemfile", ...), so a rootbundle installstill pulled in the full OSS Rails/dev/test/runtime stack instead of only the lint gems.This makes the root
Gemfilea pure lint/tooling bundle for RuboCop (and the repo-root benchmark script-spec runner), without dragging in package test/runtime dependencies.What changed
Gemfile: Removed theeval_gemfile react_on_rails/Gemfile, added an explicitsource "https://rubygems.org", and declared only the gems the root.rubocop.ymlneeds —rubocop,rubocop-performance,rubocop-rspec(therequire:plugins) — pinned to the versions the package locks resolve (1.61.0/1.20.2/2.31.0). Also added barerspec (~> 3.13)sobenchmark.yml'sbundle exec rspec benchmarks/specstep (which Centralize RuboCop ownership in root bundle #3836 wired to the root bundle) keeps working. Those specs deliberately load no Rails and noreact_on_railsgem (seebenchmarks/spec/spec_helper.rb), so barerspec— not the package'srspec-railsstack — is all the root bundle needs.Gemfile.lock: Regenerated. Now resolves 25 lint/tooling gems (down from ~150+).AGENTS.md: Updated the install note to describe the root Gemfile as a lint/tooling-only bundle.Root lock now contains vs. excludes
react_on_railsPATH gemPLATFORMSkeepsruby/arm64-darwin/x86_64-linux, soscript/check-gemfile-lock-platformsstays green.CI lint steps already point
BUNDLE_GEMFILEat the rootGemfile(from #3836), so the OSS package run (cd react_on_rails && bundle exec rubocop), the Pro run, the benchmarks lint, and the benchmark script specs all keep using the one committed root lock — now without installing the Rails/test stack.Acceptance criteria
Gemfile.lock).bundle installfor lint-only workflows avoids installing unnecessary Rails/test dependencies.Verification (local, Ruby 3.3.7, Bundler 4.0.10)
Note: bare
bundle exec rubocopfrom the repo root (the broad diagnostic sweep, not a CI step) reports 40 offenses, all confined toreact_on_rails/spike/3313_prism_gemfile_rewriter/. This is pre-existing — the package.rubocop.ymlexcludesspike/**/*, the root config does not, and this PR does not touch.rubocop.ymlor the RuboCop versions. No CI step runs the bare root sweep.Fixes #3837
🤖 Generated with Claude Code
Note
Low Risk
Tooling and lockfile scope only; runtime and package test deps are unchanged on package Gemfiles. Main risk is contributors expecting a root bundle install to install the full gem stack.
Overview
The root
Gemfileno longereval_gemfilethereact_on_railspackage Gemfile. It is now an explicit lint/tooling-only bundle: pinned RuboCop plugins, an explicitbenchmarkgem (for Ruby ≥ 3.5), and barerspec (~> 3.13)for repo-rootbenchmarks/spec(no Rails/rspec-rails).Gemfile.lockwas regenerated accordingly—roughly 25 gems instead of the full Rails/test/runtime graph (Rails, Shakapacker, PATHreact_on_rails, etc. stay on package Gemfiles).AGENTS.mddocuments that rootbundle installis for RuboCop + benchmark script specs only, not the package stack. CI paths that already setBUNDLE_GEMFILEto the rootGemfilefor RuboCop (and benchmark specs) keep the same contract with a lighter install.Reviewed by Cursor Bugbot for commit 9b7c081. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Documentation
Chores