Skip to content

Commit 9bd5482

Browse files
authored
Centralize RuboCop ownership in root bundle (#3836)
Fixes #3825. ## Summary - Add a committed root `Gemfile.lock` and make the top-level `Gemfile` own the single project RuboCop version (`rubocop 1.61.0` plus the existing Performance/RSpec plugins). - Remove RuboCop gems from the shared package development dependency file and regenerate package lockfiles so RuboCop only appears in the root lock. - Point local lint tasks, package scripts, CI Ruby lint jobs, and lint docs at the root bundle for RuboCop while leaving package Gemfiles in charge of RBS/test/runtime bundles. - Update the benchmark workflow setup and release lockfile update path now that the root bundle is a committed CI input. ## Duplicate / prior-work search - #3825 is the open target issue; latest maintainer direction is: "Top level Gemfile/Gemfile.lock should have one version of rubocop for the project." - #2214 is closed and explicitly superseded by #3825. - #3827 is merged docs-only follow-up and did not implement the root Gemfile/lock ownership change. - `gh issue list --search "rubocop Gemfile Gemfile.lock" --state all --limit 20` found no open duplicate implementation issue. #2224 is related to bundler caching, not a duplicate of this ownership change. - `gh pr list --search "rubocop Gemfile Gemfile.lock" --state all --limit 20` found #3827 plus unrelated historical PRs, with no open duplicate PR for this implementation. ## Validation - `bundle install` at repo root -> pass. - `bundle install` in `react_on_rails`, `react_on_rails_pro`, and `react_on_rails_pro/spec/dummy` -> pass. - `bundle exec rubocop --version` -> `1.61.0`. - `(cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop --version)` -> `1.61.0`. - `(cd react_on_rails_pro && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop --version)` -> `1.61.0`. - `(cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop)` -> pass, 214 files, no offenses. - `(cd react_on_rails_pro && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop --ignore-parent-exclusion)` -> pass, 228 files, no offenses; existing RuboCop AST `EnsureNode#body` deprecation warnings printed. - `bundle exec rake lint:rubocop` from root -> pass. - `(cd react_on_rails_pro && bundle exec rake lint:rubocop)` -> pass. - `(cd react_on_rails && bundle exec rake rbs:validate)` -> pass. - `(cd react_on_rails_pro && bundle exec rake rbs:validate)` -> pass. - `bundle exec rspec benchmarks/spec` -> pass, 238 examples, 0 failures. - `ruby script/check-gemfile-lock-platforms` -> pass, all 6 lockfiles include `x86_64-linux`. - `pnpm exec prettier --check AGENTS.md internal/contributor-info/linters.md .github/workflows/lint-js-and-ruby.yml .github/workflows/pro-test-package-and-gem.yml .github/workflows/benchmark.yml react_on_rails_pro/package-scripts.yml` -> pass. - `ruby -e 'require "yaml"; ARGV.each { |path| YAML.load_file(path); puts "parsed #{path}" }' .github/workflows/lint-js-and-ruby.yml .github/workflows/pro-test-package-and-gem.yml .github/workflows/benchmark.yml react_on_rails_pro/package-scripts.yml` -> pass. - `actionlint` -> pass. - `yamllint .github/` -> not run; `yamllint` is not installed in this worker environment. - `script/ci-changes-detector origin/main` -> recommends broad Ruby/JS/dummy/Pro/CI coverage and benchmark jobs. - `git diff --check origin/main...HEAD` -> pass. - `autoreview --mode branch --base origin/main` -> clean, no accepted/actionable findings, overall patch correct 0.84. - Pre-push hook -> pass: branch Ruby RuboCop and Markdown link checks. ## Workflow Change Audit - Secret references: no added or changed secret references. Existing benchmark workflow secret references are unchanged. - `permissions:`: no changes. - `on:` triggers: no changes. - Third-party actions: no new third-party actions and no third-party action version changes. The benchmark workflow now uses the existing local `./.github/actions/setup-bundle`; lint workflows add existing local setup-bundle steps for the root bundle. ## CI label recommendation Labels: `full-ci, benchmark`. Reason: this changes committed Bundler lock ownership, Ruby lint entry points, CI workflow setup, package lockfiles, and the benchmark workflow. The local CI detector also recommends broad suites and benchmark jobs. ## Known residual risk / UNKNOWN - UNKNOWN whether maintainers eventually want the top-level `Gemfile` to be a pure lint-only bundle. This PR keeps the existing root Gemfile behavior of delegating to `react_on_rails/Gemfile` while making RuboCop itself root-owned, which is the narrowest change that follows the maintainer comment without breaking root rake/development usage. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches committed lockfiles, multiple CI workflows, and every RuboCop invocation path; wrong `BUNDLE_GEMFILE` wiring could break lint locally or in CI without affecting runtime gem behavior. > > **Overview** > **RuboCop is now owned by the root `Gemfile` / committed `Gemfile.lock`**, with pinned `rubocop` 1.61.0 and plugins; RuboCop gems were removed from `Gemfile.shared_dev_dependencies` and from OSS/Pro package lockfiles. > > **Lint entry points** (CI, `bin/ci-local`, rake `lint` tasks, Pro package scripts, dummy `bin/rubocop`, `AGENTS.md`, contributor docs) now run RuboCop via `BUNDLE_GEMFILE` pointing at the repo root. OSS/Pro **RBS validation** still uses each package’s Gemfile. > > **CI** adds root `setup-bundle` for lint workflows; benchmark job drops manual `bundle install` in favor of root `setup-bundle` for benchmark specs. **Release** runs `bundle install` at the monorepo root when refreshing lockfiles. Root `Gemfile.lock` is no longer gitignored. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit e4910a0. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> ## Agent Merge Confidence Mode: accelerated-rc Score: 9/10 Auto-merge recommendation: yes Affected areas: RuboCop ownership, Bundler lockfiles, CI lint wiring, Pro/core lint boundary, benchmark workflow CI detector: `script/ci-changes-detector origin/main` -> broad Ruby/JS/dummy/Pro/CI coverage and benchmark jobs recommended Validation run: - `cd react_on_rails_pro && BUNDLE_GEMFILE="$(git rev-parse --show-toplevel)/Gemfile" bundle exec rubocop --ignore-parent-exclusion rakelib/lint.rake rakelib/task_helpers.rb` -> pass, RuboCop 1.61.0 - `script/ci-changes-detector origin/main` -> full suite + benchmarks recommended - Full GitHub checks for `e4910a0f889ef7b4b07b658c97e1c921ea381e4a` -> 50 pass, 4 expected skips Review/check gate: - Claude review: complete for `e4910a0f889ef7b4b07b658c97e1c921ea381e4a`, no unresolved review threads - Cursor Bugbot: complete for `e4910a0f889ef7b4b07b658c97e1c921ea381e4a` - GitHub checks: complete for `e4910a0f889ef7b4b07b658c97e1c921ea381e4a`; skipped benchmark confirmation/report rows and docs-format-check are expected non-running paths Known residual risk: low; root bundle ownership is broad but current-head full CI, Pro lint, and benchmark suites passed Finalized by: Claude Code Review check `claude-review` SUCCESS for `e4910a0f889ef7b4b07b658c97e1c921ea381e4a` (GitHub Actions job 80360583030)
1 parent 28702ff commit 9bd5482

22 files changed

Lines changed: 790 additions & 197 deletions

File tree

.github/workflows/benchmark.yml

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -135,20 +135,12 @@ jobs:
135135
shell: bash
136136
# Only pay the Bundler setup when benchmarks will run or the scripts
137137
# themselves changed; unrelated PRs keep this job stdlib-only and fast.
138-
# TODO: when fixing #2214 this should switch to the new root Gemfile.
139-
- name: Setup Ruby
138+
- name: Install gems for benchmark specs
140139
if: steps.benchmark-matrices.outputs.run_benchmark_suites == 'true' || steps.detect.outputs.benchmarks_changed == 'true'
141-
uses: ./.github/actions/setup-ruby
140+
uses: ./.github/actions/setup-bundle
142141
with:
142+
working-directory: .
143143
ruby-version: ${{ env.RUBY_VERSION }}
144-
# The benchmark specs only need rspec, which the root Gemfile provides (it
145-
# delegates to react_on_rails/Gemfile). The setup-bundle action can't manage
146-
# it because there is no committed root Gemfile.lock yet, so install manually.
147-
# TODO(#2214): once a root Gemfile.lock is committed, replace this step with
148-
# the setup-bundle action (working-directory: ".") and delete it.
149-
- name: Install gems for benchmark specs
150-
if: steps.benchmark-matrices.outputs.run_benchmark_suites == 'true' || steps.detect.outputs.benchmarks_changed == 'true'
151-
run: bundle install --jobs 4 --retry 3
152144
# Validate the benchmark scripts (matrix generation, route discovery) before
153145
# any benchmark runs: if this logic is wrong, the matrix and routes feeding
154146
# the run are wrong, so the results would be invalid. Running it here (rather

.github/workflows/lint-js-and-ruby.yml

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,12 @@ jobs:
151151
echo "Bundler version: "; bundle --version
152152
- name: Check Gemfile.lock platforms
153153
run: ruby script/check-gemfile-lock-platforms
154+
- name: Install Ruby Gems for root lint tools
155+
uses: ./.github/actions/setup-bundle
156+
with:
157+
working-directory: .
158+
ruby-version: '4.0'
159+
install-libyaml: 'false'
154160
- name: Install Ruby Gems for package
155161
uses: ./.github/actions/setup-bundle
156162
with:
@@ -160,17 +166,17 @@ jobs:
160166
- name: Install Node modules with pnpm
161167
run: pnpm install --frozen-lockfile
162168

163-
# Pin the package Gemfile explicitly for parity with pro-lint.yml and to
164-
# stay robust if the package and dummy Gemfiles ever diverge.
169+
# RuboCop is owned by the root Gemfile/Gemfile.lock so OSS and Pro use one
170+
# locked lint version. Keep package bundles for package-specific tasks.
165171
- name: Lint Ruby
166172
env:
167-
BUNDLE_GEMFILE: ${{ github.workspace }}/react_on_rails/Gemfile
173+
BUNDLE_GEMFILE: ${{ github.workspace }}/Gemfile
168174
run: cd react_on_rails && bundle exec rubocop
169175
# The benchmarks/ scripts live outside the gem, so the package rubocop run
170176
# above doesn't see them. Lint them from the repo root (root .rubocop.yml).
171177
- name: Lint benchmark scripts
172178
env:
173-
BUNDLE_GEMFILE: ${{ github.workspace }}/react_on_rails/Gemfile
179+
BUNDLE_GEMFILE: ${{ github.workspace }}/Gemfile
174180
run: bundle exec rubocop benchmarks
175181
- name: Validate RBS type signatures
176182
env:

.github/workflows/pro-test-package-and-gem.yml

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,11 +178,18 @@ jobs:
178178
working-directory: .
179179
run: pnpm --filter react-on-rails-pro build
180180

181-
# These steps need the Pro package Gemfile (which carries rubocop/rbs),
182-
# so pin it via absolute path — robust to changes in defaults.run.
181+
- name: Install Ruby Gems for root lint tools
182+
uses: ./.github/actions/setup-bundle
183+
with:
184+
working-directory: .
185+
ruby-version: ${{ env.RUBY_VERSION }}
186+
install-libyaml: 'false'
187+
188+
# RuboCop is owned by the root Gemfile/Gemfile.lock so OSS and Pro use one
189+
# locked lint version. Keep the Pro package Gemfile for RBS.
183190
- name: Lint Ruby
184191
env:
185-
BUNDLE_GEMFILE: ${{ github.workspace }}/react_on_rails_pro/Gemfile
192+
BUNDLE_GEMFILE: ${{ github.workspace }}/Gemfile
186193
run: bundle exec rubocop --ignore-parent-exclusion
187194

188195
- name: Validate RBS type signatures

.gitignore

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
# Root Gemfile.lock (delegates to react_on_rails/Gemfile, lock is there)
2-
/Gemfile.lock
3-
41
.bundle/
52
/.yardoc
63
/_yardoc/

AGENTS.md

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,26 @@ If there is a conflict, `AGENTS.md` wins.
3737

3838
```bash
3939
# Install dependencies
40+
# The committed root Gemfile.lock is generated with Bundler 4.0.10; use Bundler
41+
# 4.0.10 or newer before running root bundle commands.
4042
bundle && pnpm install
4143

44+
# After changing react_on_rails/Gemfile or Gemfile.shared_dev_dependencies,
45+
# also run bundle install at the repo root to keep the root Gemfile.lock synced.
46+
4247
# Build TypeScript → JavaScript
4348
pnpm run build
4449

4550
# Lint (MANDATORY before every commit)
46-
(cd react_on_rails && bundle exec rubocop) # OSS Ruby lint — CI-equivalent
51+
(cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop) # OSS Ruby lint — CI-equivalent
4752
# Pro Ruby lint — CI-equivalent when Pro files or RuboCop config change
48-
(cd react_on_rails_pro && bundle exec rubocop --ignore-parent-exclusion)
53+
(cd react_on_rails_pro && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop --ignore-parent-exclusion)
4954
pnpm run lint # JS/TS via ESLint
5055
pnpm start format.listDifferent # Check Prettier formatting
5156
rake lint # All linting (Ruby + JS + formatting)
5257

5358
# Optional Ruby diagnostic from the repo root (not the CI contract)
54-
bundle exec rubocop
59+
BUNDLE_GEMFILE=Gemfile bundle exec rubocop
5560

5661
# Auto-fix formatting
5762
rake autofix # Preferred for all formatting
@@ -277,13 +282,13 @@ For small, focused PRs (roughly 5 files changed or fewer and one clear purpose):
277282

278283
### Always
279284

280-
- Run the CI-equivalent Ruby lint before committing, not the root `bundle exec rubocop`:
285+
- Run the CI-equivalent Ruby lint before committing:
281286
```bash
282-
(cd react_on_rails && bundle exec rubocop)
287+
(cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop)
283288
# Also run when touching Pro Ruby or RuboCop config:
284-
(cd react_on_rails_pro && bundle exec rubocop --ignore-parent-exclusion)
289+
(cd react_on_rails_pro && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop --ignore-parent-exclusion)
285290
```
286-
Root `bundle exec rubocop` is a broad local sweep, not the CI contract.
291+
The root `Gemfile` owns the RuboCop version; package directories own their test and RBS bundles.
287292
- Use `pnpm` for all JS operations — never `npm` or `yarn`
288293
- Use `bundle exec` for Ruby commands
289294
- Ensure all files end with a newline

Gemfile

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
# frozen_string_literal: true
22

3-
# Root Gemfile for developer convenience
4-
# Delegates to the react_on_rails gem for development
3+
# Root Gemfile for developer convenience and RuboCop tooling.
4+
# RuboCop is owned here so the monorepo uses one locked version.
55

66
# Use the open-source gem's Gemfile
77
eval_gemfile File.expand_path("react_on_rails/Gemfile", __dir__)
8+
9+
group :development, :test do
10+
gem "rubocop", "1.61.0", require: false
11+
gem "rubocop-performance", "1.20.2", require: false
12+
gem "rubocop-rspec", "2.31.0", require: false
13+
end

0 commit comments

Comments
 (0)