Skip to content

Conversation

@alexeyr-ci2
Copy link
Collaborator

@alexeyr-ci2 alexeyr-ci2 commented Dec 1, 2025

Summary

Extracted from #1868 because it adds spec/performance/bench.rb and the default Rubocop config doesn't fit our repo.

Pull Request checklist

  • Add/update test to cover these changes
  • Update documentation
  • Update CHANGELOG file

Summary by CodeRabbit

  • Chores
    • Centralized linting with a root-level RuboCop config and subpackages inheriting shared rules for consistency.
    • Simplified per-package overrides, reducing local rule toggles and maintenance overhead.
    • Formatter ignore behavior adjusted: most lint config files are now formatted automatically; the pro package keeps an ignore entry for its RuboCop config.
    • Added a development/test linting helper dependency to support the new centralized setup.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 1, 2025

Walkthrough

Adds a root .rubocop.yml, consolidates package RuboCop configs to inherit from it, updates Prettier ignore rules so root .rubocop.yml is formatted while the pro package keeps its .rubocop.yml ignored, and adds rubocop-rails-omakase to development dependencies.

Changes

Cohort / File(s) Summary
Prettier ignore updates
/.prettierignore, react_on_rails_pro/.prettierignore
Removed the exception for .rubocop.yml in the repository root .prettierignore so the root RuboCop config is processed by Prettier; added .rubocop.yml (with a comment) to react_on_rails_pro/.prettierignore to keep the pro package file ignored.
Root RuboCop config
/.rubocop.yml
New global RuboCop config requiring rubocop-performance and rubocop-rspec, sets AllCops (EnableNewCops, DisplayCopNames, TargetRubyVersion 3.0), Include/Exclude patterns, Style and Metrics settings, several RSpec cop toggles, and enforces Style/FrozenStringLiteralComment.
Package RuboCop inheritance
react_on_rails/.rubocop.yml, react_on_rails_pro/.rubocop.yml
Both package configs now inherit from the root (inherit_from: ../.rubocop.yml) and remove most local per-cop overrides, keeping only a narrowed set of excludes and a few package-specific exceptions.
Development dependency
react_on_rails/Gemfile.development_dependencies
Adds gem "rubocop-rails-omakase", "~>1.0.0", require: false to the :development, :test group.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Check that inheritance paths (inherit_from) resolve correctly in CI and local tooling.
  • Verify that re-enabled/enforced cops from the root config don't trigger unexpected failures in package code or CI.
  • Confirm Prettier formatting on the root .rubocop.yml (ensure no ERB issues) and that the pro package remains intentionally ignored.

Possibly related PRs

Suggested labels

full-ci

Suggested reviewers

  • AbanoubGhadban

Poem

🐰 I hopped the repo, tidy and spry,
One root rule now watches the sky.
Pro keeps its hush, Prettier paints the rest,
Gems added, configs set to their nest.
Thump-thump — linted, neat, and blessed.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Unify Rubocop config' directly and accurately describes the main objective of this pull request, which consolidates RuboCop configurations across the monorepo by creating a root .rubocop.yml and updating subproject configurations to inherit from it.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch alexeyr/rubocop-config

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Dec 1, 2025

Code Review: Unify RuboCop Config

Summary

This PR successfully consolidates the RuboCop configuration by extracting common settings to a root .rubocop.yml file, with react_on_rails/ and react_on_rails_pro/ inheriting via inherit_from. This is excellent DRY practice and eliminates configuration drift between the two packages.

Strengths

  1. Proper DRY Architecture: Moving shared settings to root and using inherit_from is the correct approach for a monorepo
  2. Good Exclusion Cleanup: Removed redundant exclusions from child configs that are already in root
  3. Prettier Protection: Correctly added .rubocop.yml to react_on_rails_pro/.prettierignore since it contains ERB
  4. Preserved Customizations: Package-specific overrides are correctly kept in child configs

Issues and Concerns

1. ERB in RuboCop Config - Potential Compatibility Issue

The react_on_rails_pro/.rubocop.yml file contains ERB template code that dynamically generates exclusion patterns from git-ignored files. RuboCops ERB support is not guaranteed across all versions and executes shell commands during config parsing, which could be slow or error-prone.

Recommendation: Consider using static exclusion patterns instead, or document why ERB is necessary and test with minimum supported RuboCop version.

2. Missing Root TargetRubyVersion Validation

The root config sets TargetRubyVersion: 3.0. Please verify this matches the gemspecs required_ruby_version and the minimum CI configuration.

3. Configuration Path Testing

Per CLAUDE.md guidance on Managing File Paths, configuration changes should be tested from all locations to ensure inheritance works correctly.

Testing Checklist

Before merging, please confirm:

  • bundle exec rubocop passes from repo root
  • cd react_on_rails && bundle exec rubocop passes
  • cd react_on_rails_pro && bundle exec rubocop passes
  • ERB in Pro config parses correctly
  • Pre-commit hooks still work
  • CI passes

Overall Assessment

Verdict: Approve with minor concerns

This is a solid refactoring that improves maintainability. The ERB concern is the main issue to address. Once verified that ERB works reliably and the testing checklist is complete, this is good to merge.

- 'lib/generators/react_on_rails/install_generator.rb' # Generator setup methods require comprehensive error handling

Metrics/CyclomaticComplexity:
Max: 7
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This remains here because it doesn't match exactly with RORP

Metrics/CyclomaticComplexity:
Max: 8
but I could just pick the greater limit in cases like that (also Metrics/ClassLength 150 here vs. 140 in RORP etc.)

@alexeyr-ci2 alexeyr-ci2 force-pushed the alexeyr/rubocop-config branch from 0812423 to b7cfbb2 Compare December 1, 2025 16:01
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
react_on_rails_pro/.rubocop.yml (1)

6-8: ERB-based dynamic git exclusions are clever but performance-sensitive; consider CI/CD implications.

RuboCop processes .rubocop.yml as ERB before YAML parsing, allowing arbitrary Ruby code embedding. Lines 6-8 execute git status --ignored --porcelain on each RuboCop invocation to exclude git-ignored files dynamically.

Pros:

  • Automatically respects .gitignore without manual maintenance

Cons:

  • Runs a shell command on every RuboCop check (performance impact, especially in large repos)
  • May fail in non-git environments or shallow clones
  • Could behave unexpectedly in CI/CD if git state differs from local

Consider adding error handling or an environment variable escape hatch if git state is unavailable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0812423 and b7cfbb2.

📒 Files selected for processing (5)
  • .prettierignore (0 hunks)
  • .rubocop.yml (1 hunks)
  • react_on_rails/.rubocop.yml (1 hunks)
  • react_on_rails_pro/.prettierignore (1 hunks)
  • react_on_rails_pro/.rubocop.yml (1 hunks)
💤 Files with no reviewable changes (1)
  • .prettierignore
🚧 Files skipped from review as they are similar to previous changes (2)
  • react_on_rails_pro/.prettierignore
  • .rubocop.yml
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to **/*.{rb,rake} : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: ALWAYS let Prettier and RuboCop handle ALL formatting - never manually format
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to **/*.{rb,rake} : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to lib/react_on_rails/*.rb : Create RBS signature files for new Ruby files in `lib/react_on_rails/` by adding corresponding `.rbs` file in `sig/react_on_rails/`, adding to Steepfile, and validating before committing

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to sig/react_on_rails/**/*.rbs : In RBS type signature files, define type signatures that correspond to Ruby files in `lib/react_on_rails/`

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Exclude `/coverage`, `/tmp`, `/gen-examples`, `/packages/react-on-rails/lib`, `/node_modules`, `/spec/dummy/node_modules`, `/spec/dummy/tmp`, `/spec/dummy/app/assets/webpack`, `/spec/dummy/log`, `/spec/dummy/e2e/playwright-report`, `/spec/dummy/test-results` from IDE indexing

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: ALWAYS let Prettier and RuboCop handle ALL formatting - never manually format

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to spec/dummy/e2e/playwright/**/*.spec.js : Use Rails helpers (`appFactories`, `appEval`) in Playwright tests instead of UI-based setup to leverage factory_bot and arbitrary Ruby code execution

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-02-13T14:29:49.267Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.

Applied to files:

  • react_on_rails_pro/.rubocop.yml
  • react_on_rails/.rubocop.yml
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation is handled in deeper level calls of the React on Rails Pro codebase, so it doesn't need to be validated again in the `rsc_payload_react_component` helper method.

Applied to files:

  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Runtime type checking via RBS is enabled by default in test runs; disable with `DISABLE_RBS_RUNTIME_CHECKING=true` only for rapid development iterations

Applied to files:

  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to lib/tasks/*.rake : Rails::Engine rake tasks should NOT be explicitly loaded via `rake_tasks` block; instead, place them in `lib/tasks/*.rake` where Rails::Engine automatically loads them

Applied to files:

  • react_on_rails/.rubocop.yml
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-10-08T20:53:47.076Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
  before do
    original_value = ENV["VARIABLE_NAME"]
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
  end

  after do
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
    ENV["VARIABLE_NAME"] = original_value
  end

  it "tests something" do
    # Your test code here
  end
end
```
This practice ensures test isolation and reliability.

Applied to files:

  • react_on_rails/.rubocop.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: examples (3.4, latest)
  • GitHub Check: rspec-package-tests (3.4, latest)
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: pro-lint-js-and-ruby
  • GitHub Check: build
  • GitHub Check: claude-review
🔇 Additional comments (2)
react_on_rails_pro/.rubocop.yml (1)

1-2: Root config inheritance established; verify same root file is properly configured.

This file now inherits from the same root config via ../. rubocop.yml. Same verification needed as react_on_rails/.rubocop.yml:

  1. Root config exists and contains all necessary cop definitions
  2. Root config appropriately handles the pro package's stricter or looser limits (e.g., Metrics/ClassLength: Max: 140 here vs. 150 in the open-source package)
  3. Performance benchmark file handling is configured
react_on_rails/.rubocop.yml (1)

1-2: Root RuboCop configuration is correctly set up and inheritance is working as expected.

The root .rubocop.yml exists at the repository root and properly defines all necessary cop configurations including Layout/LineLength (Max: 120), Style/StringLiterals, and others. The relative path ../.rubocop.yml in react_on_rails/.rubocop.yml correctly resolves to the root config. The spec/performance/bench.rb file will be included in RuboCop checks via the **/*.rb include pattern in the root config's AllCops section, so no additional configuration is needed.

@claude
Copy link

claude bot commented Dec 1, 2025

Code Review

Summary

This PR successfully unifies RuboCop configuration across the monorepo by creating a root-level config that both react_on_rails/ and react_on_rails_pro/ inherit from. This is a good refactoring that reduces duplication and ensures consistency.

✅ Strengths

  1. DRY Principle Applied: Consolidating shared RuboCop rules into a root config eliminates ~150 lines of duplicate configuration
  2. Proper Inheritance: Both sub-packages correctly use inherit_from: ../.rubocop.yml to extend the base config
  3. Prettier Ignore Handling: Smart handling of ERB in Pro's .rubocop.yml by keeping it in .prettierignore while removing the blanket exclusion at root
  4. Maintained Flexibility: Sub-packages can still override rules as needed (e.g., Metrics/CyclomaticComplexity differs between packages)

🔍 Issues Found

1. Inconsistent AllCops Setting (Minor)

Location: react_on_rails_pro/.rubocop.yml:5

AllCops:
  EnableNewCops: true  # Note: "Enable" not "New"

The root config uses NewCops: enable but Pro uses EnableNewCops: true. While RuboCop may accept both, this inconsistency could cause confusion.

Recommendation: Use NewCops: enable everywhere for consistency, or verify this wasn't intentional.

2. Missing Exclusion Pattern (Potential Bug)

Location: Root .rubocop.yml:26

The root config excludes gen-examples/**/* but the old react_on_rails/.rubocop.yml excluded gen-examples/examples/**/* (more specific).

Question: Does gen-examples/ contain files outside of examples/ that should be linted? If so, the new pattern is more correct. If not, the old pattern was more precise.

3. TargetRubyVersion Precision

Location: Root .rubocop.yml:11

TargetRubyVersion: 3.0

The old react_on_rails/.rubocop.yml had 3.0.0 (patch version specified). While both work, double-check this matches your actual minimum supported Ruby version per CLAUDE.md (which mentions 3.0 as the minimum).

📋 Testing Recommendations

Per CLAUDE.md's requirements, before merging:

  1. Run RuboCop on both packages:

    bundle exec rubocop
    cd react_on_rails_pro && bundle exec rubocop
  2. Verify no new offenses were introduced by the config changes

  3. Test the ERB in Pro's config still works:

    cd react_on_rails_pro
    git status --ignored --porcelain | grep '^\!\! ' # Should output ignored files
    bundle exec rubocop --debug # Should process exclusions correctly
  4. Verify Prettier respects the new ignore rules:

    pnpm run format.listDifferent

💡 Suggestions for Future

  1. Document the ERB usage: Consider adding a comment in react_on_rails_pro/.rubocop.yml explaining why ERB is used (to dynamically exclude git-ignored files)

  2. Consolidate Prettier ignores: The Pro package now has a specific .prettierignore for .rubocop.yml, but the root already excludes all *.yml. This works but creates a dependency on order of operations.

🎯 Verdict

Approve with minor recommendations. The refactoring is sound and follows the monorepo structure correctly. The inconsistencies noted are minor and could be addressed in a follow-up if needed. The CI checks passing will confirm no regressions were introduced.

Great work on reducing configuration duplication! 🚀

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
react_on_rails_pro/.rubocop.yml (1)

7-9: Consider adding error handling or static fallback for the git-based ERB exclusions.

The ERB pattern dynamically excludes git-ignored files, which is a valid and officially supported RuboCop feature. However, if the git status command fails (e.g., in shallow clones without git history, or CI environments without git), the entire RuboCop config fails to load with no graceful fallback. While this is unlikely in typical development or your current CI setup (GitHub Actions provides git), defensive coding would suggest either:

  • Wrapping the backtick command with error handling: <% \git status --ignored --porcelain 2>/dev/null`.lines.grep(/^!! /).each do |path| %>`
  • Or providing a static fallback list for environments where git is unavailable

The performance impact is minimal (git runs once per RuboCop invocation, not per file).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b7cfbb2 and c029e91.

📒 Files selected for processing (1)
  • react_on_rails_pro/.rubocop.yml (1 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to **/*.{rb,rake} : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: ALWAYS let Prettier and RuboCop handle ALL formatting - never manually format
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to **/*.{rb,rake} : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to lib/react_on_rails/*.rb : Create RBS signature files for new Ruby files in `lib/react_on_rails/` by adding corresponding `.rbs` file in `sig/react_on_rails/`, adding to Steepfile, and validating before committing

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to sig/react_on_rails/**/*.rbs : In RBS type signature files, define type signatures that correspond to Ruby files in `lib/react_on_rails/`

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: ALWAYS let Prettier and RuboCop handle ALL formatting - never manually format

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-10-23T17:22:01.074Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to spec/dummy/e2e/playwright/**/*.spec.js : Use Rails helpers (`appFactories`, `appEval`) in Playwright tests instead of UI-based setup to leverage factory_bot and arbitrary Ruby code execution

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-02-13T14:29:49.267Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.

Applied to files:

  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Exclude `/coverage`, `/tmp`, `/gen-examples`, `/packages/react-on-rails/lib`, `/node_modules`, `/spec/dummy/node_modules`, `/spec/dummy/tmp`, `/spec/dummy/app/assets/webpack`, `/spec/dummy/log`, `/spec/dummy/e2e/playwright-report`, `/spec/dummy/test-results` from IDE indexing

Applied to files:

  • react_on_rails_pro/.rubocop.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: examples (3.4, latest)
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: rspec-package-tests (3.4, latest)
  • GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22, latest)
  • GitHub Check: pro-lint-js-and-ruby
  • GitHub Check: build
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: claude-review
🔇 Additional comments (3)
react_on_rails_pro/.rubocop.yml (3)

5-5: Good practice: EnableNewCops: true aligns with staying current.

This setting ensures new cops introduced in future RuboCop versions are enabled by default, supporting consistent linting standards.


16-42: No duplication or conflicts detected—local configuration properly extends root config.

The local cop configurations in react_on_rails_pro/.rubocop.yml correctly inherit from the root config via inherit_from: ../.rubocop.yml. The local settings (Metrics/CyclomaticComplexity, Metrics/ClassLength, RSpec/NestedGroups, RSpec/BeforeAfterAll, RSpec/MultipleDescribes, Style/GlobalVars) are unique to this subdirectory and do not duplicate any root-level configurations. The root config defines different Metrics rules (AbcSize, PerceivedComplexity, ParameterLists, MethodLength, ModuleLength) and RSpec rules without these specific exclusions, so the local overrides and exclusion lists are intentional and conflict-free.


1-2: Root .rubocop.yml exists and is properly configured. The inheritance in react_on_rails_pro/.rubocop.yml is correctly set up with inherit_from: ../.rubocop.yml. The root config includes standard exclusions that adequately cover project artifacts. No configuration issues detected.

Note: spec/performance/bench.rb does not currently exist in the repository, so the stated PR objective cannot be verified against the actual codebase state.

@alexeyr-ci2 alexeyr-ci2 force-pushed the alexeyr/rubocop-config branch from c029e91 to 5ff29fb Compare December 1, 2025 18:30
@coderabbitai coderabbitai bot added the full-ci label Dec 1, 2025
@claude
Copy link

claude bot commented Dec 1, 2025

Code Review - RuboCop Config Unification

Thank you for unifying the RuboCop configuration! This is a valuable cleanup that reduces duplication.

✅ Strengths

  1. Good consolidation: Moving shared configuration to root level reduces duplication
  2. Proper inheritance: Both packages now use inherit_from correctly
  3. Preserved package-specific overrides: Package-specific exceptions are maintained
  4. Prettier config updated appropriately: Removed .rubocop.yml from root .prettierignore, kept it in Pro package

⚠️ Issues & Concerns

1. Configuration Inconsistency: NewCops vs EnableNewCops

Root config uses NewCops: enable (line 8) but react_on_rails_pro/.rubocop.yml overrides with EnableNewCops: true (line 5). These are different settings - NewCops is modern, EnableNewCops is legacy. Remove EnableNewCops from Pro config since it is redundant.

2. Missing Gem Requirement

rubocop-rails-omakase was added to Gemfile but not required in .rubocop.yml. Either add it to require: section if using its cops, or remove from Gemfile if not needed yet.

3. ERB Template in Pro Config

Pro .rubocop.yml uses ERB to run git status dynamically. This makes linting non-deterministic and could be slow. Consider replacing with explicit exclusion patterns.

4. CyclomaticComplexity Relaxed

Root config sets Max: 8 but old react_on_rails config had Max: 7 (stricter). This relaxes the limit from 7 to 8. Is this intentional?

5. RSpec/NestedGroups Relaxed

Root config sets Max: 5 but old react_on_rails config had Max: 4 (stricter). This allows deeper nesting. Is this intentional?

6. Metrics/ClassLength Change

Root uses Max: 150, Pro had Max: 140 before. Pro now allows slightly longer classes.

Testing Checklist

  • Run bundle exec rubocop from root and each package
  • Check for new violations from relaxed limits
  • Verify inheritance works correctly
  • Test with fresh bundle

Overall Assessment

This is a good refactoring. Main concerns:

  1. Configuration value inconsistencies may indicate unintended changes
  2. Unclear purpose of rubocop-rails-omakase dependency
  3. Dynamic ERB-based exclusions in Pro package

Recommendation: Address inconsistencies, verify no new violations, and document intentional policy changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
react_on_rails/.rubocop.yml (1)

1-80: File must end with a newline character.

The file is missing a trailing newline after line 80, which violates the coding guideline.

As per coding guidelines, add a newline at the end of the file:

 RSpec/StubbedMock:
   Exclude:
     - 'spec/react_on_rails/dev/**/*_spec.rb'  # Dev module tests use mixed stub/mock patterns
+
react_on_rails_pro/.rubocop.yml (1)

1-33: File must end with a newline character.

The file is missing a trailing newline after line 33, which violates the coding guideline.

As per coding guidelines, add a newline at the end of the file:

 Style/GlobalVars:
   Exclude:
     - 'spec/dummy/config/environments/development.rb'
+
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c029e91 and 5ff29fb.

⛔ Files ignored due to path filters (1)
  • react_on_rails/Gemfile.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • .prettierignore (0 hunks)
  • .rubocop.yml (1 hunks)
  • react_on_rails/.rubocop.yml (1 hunks)
  • react_on_rails/Gemfile.development_dependencies (1 hunks)
  • react_on_rails_pro/.prettierignore (1 hunks)
  • react_on_rails_pro/.rubocop.yml (1 hunks)
💤 Files with no reviewable changes (1)
  • .prettierignore
🚧 Files skipped from review as they are similar to previous changes (1)
  • react_on_rails_pro/.prettierignore
🧰 Additional context used
📓 Path-based instructions (1)
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS ensure files end with a newline character before committing/pushing

Files:

  • react_on_rails/Gemfile.development_dependencies
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to **/*.{rb,rake} : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to lib/react_on_rails/*.rb : Create RBS signature files for new Ruby files in `lib/react_on_rails/` by adding corresponding `.rbs` file in `sig/react_on_rails/`, adding to Steepfile, and validating before committing
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: ALWAYS let Prettier and RuboCop handle ALL formatting - never manually format
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package

Applied to files:

  • react_on_rails/Gemfile.development_dependencies
  • react_on_rails/.rubocop.yml
  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to lib/react_on_rails/*.rb : Create RBS signature files for new Ruby files in `lib/react_on_rails/` by adding corresponding `.rbs` file in `sig/react_on_rails/`, adding to Steepfile, and validating before committing

Applied to files:

  • react_on_rails/Gemfile.development_dependencies
  • react_on_rails/.rubocop.yml
  • react_on_rails_pro/.rubocop.yml
  • .rubocop.yml
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.

Applied to files:

  • react_on_rails/Gemfile.development_dependencies
  • react_on_rails/.rubocop.yml
  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.

Applied to files:

  • react_on_rails/Gemfile.development_dependencies
  • react_on_rails/.rubocop.yml
  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to sig/react_on_rails/**/*.rbs : In RBS type signature files, define type signatures that correspond to Ruby files in `lib/react_on_rails/`

Applied to files:

  • react_on_rails/Gemfile.development_dependencies
  • react_on_rails/.rubocop.yml
  • react_on_rails_pro/.rubocop.yml
  • .rubocop.yml
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.

Applied to files:

  • react_on_rails/Gemfile.development_dependencies
  • react_on_rails/.rubocop.yml
  • react_on_rails_pro/.rubocop.yml
  • .rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately

Applied to files:

  • react_on_rails/Gemfile.development_dependencies
  • react_on_rails/.rubocop.yml
  • react_on_rails_pro/.rubocop.yml
  • .rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to **/*.{rb,rake} : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push

Applied to files:

  • react_on_rails/Gemfile.development_dependencies
  • react_on_rails/.rubocop.yml
  • react_on_rails_pro/.rubocop.yml
  • .rubocop.yml
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation is handled in deeper level calls of the React on Rails Pro codebase, so it doesn't need to be validated again in the `rsc_payload_react_component` helper method.

Applied to files:

  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: ALWAYS let Prettier and RuboCop handle ALL formatting - never manually format

Applied to files:

  • react_on_rails/.rubocop.yml
  • react_on_rails_pro/.rubocop.yml
  • .rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Exclude `/coverage`, `/tmp`, `/gen-examples`, `/packages/react-on-rails/lib`, `/node_modules`, `/spec/dummy/node_modules`, `/spec/dummy/tmp`, `/spec/dummy/app/assets/webpack`, `/spec/dummy/log`, `/spec/dummy/e2e/playwright-report`, `/spec/dummy/test-results` from IDE indexing

Applied to files:

  • react_on_rails/.rubocop.yml
  • .rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to spec/dummy/e2e/playwright/**/*.spec.js : Use Rails helpers (`appFactories`, `appEval`) in Playwright tests instead of UI-based setup to leverage factory_bot and arbitrary Ruby code execution

Applied to files:

  • react_on_rails/.rubocop.yml
📚 Learning: 2025-02-13T14:29:49.267Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.

Applied to files:

  • react_on_rails/.rubocop.yml
  • react_on_rails_pro/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Runtime type checking via RBS is enabled by default in test runs; disable with `DISABLE_RBS_RUNTIME_CHECKING=true` only for rapid development iterations

Applied to files:

  • react_on_rails/.rubocop.yml
📚 Learning: 2025-11-25T08:05:17.804Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.804Z
Learning: Applies to lib/tasks/*.rake : Rails::Engine rake tasks should NOT be explicitly loaded via `rake_tasks` block; instead, place them in `lib/tasks/*.rake` where Rails::Engine automatically loads them

Applied to files:

  • react_on_rails/.rubocop.yml
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-10-08T20:53:47.076Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
  before do
    original_value = ENV["VARIABLE_NAME"]
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
  end

  after do
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
    ENV["VARIABLE_NAME"] = original_value
  end

  it "tests something" do
    # Your test code here
  end
end
```
This practice ensures test isolation and reliability.

Applied to files:

  • react_on_rails/.rubocop.yml
📚 Learning: 2025-10-23T17:22:01.074Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.

Applied to files:

  • react_on_rails_pro/.rubocop.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: rspec-package-tests (3.4, latest)
  • GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22, latest)
  • GitHub Check: examples (3.4, latest)
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: build
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: pro-lint-js-and-ruby
  • GitHub Check: claude-review
🔇 Additional comments (3)
.rubocop.yml (1)

1-99: Root configuration structure looks solid.

The shared RuboCop configuration provides a good baseline with reasonable cop settings, metrics thresholds, and RSpec configurations that both packages can inherit from.

react_on_rails/.rubocop.yml (1)

1-2: Good consolidation via inheritance.

Switching to inherit from the root configuration is a clean approach that reduces duplication while maintaining package-specific exceptions where needed.

react_on_rails_pro/.rubocop.yml (1)

1-2: Inheritance approach reduces duplication effectively.

Inheriting from the root config while maintaining only pro-specific exclusions is a clean consolidation.

Comment on lines +1 to +99
# Root RuboCop configuration for React on Rails monorepo
# This contains shared settings inherited by react_on_rails/ and react_on_rails_pro/
require:
- rubocop-performance
- rubocop-rspec

AllCops:
NewCops: enable
DisplayCopNames: true
SuggestExtensions: false
TargetRubyVersion: 3.0

Include:
- '**/Rakefile'
- '**/config.ru'
- 'Gemfile'
- '**/*.rb'
- '**/*.rake'

Exclude:
- '**/*.js'
- '**/node_modules/**/*'
- '**/public/**/*'
- '**/tmp/**/*'
- 'coverage/**/*'
- 'gen-examples/**/*'
- 'node_modules/**/*'
- 'spec/fixtures/**/*'
- 'spec/react_on_rails/dummy-for-generators/**/*'
- 'vendor/**/*'

Naming/FileName:
Exclude:
- '**/Gemfile'
- '**/Rakefile'

Layout/LineLength:
Max: 120

Style/StringLiterals:
EnforcedStyle: double_quotes

Style/Documentation:
Enabled: false

Style/HashEachMethods:
Enabled: true

Style/HashTransformKeys:
Enabled: true

Style/HashTransformValues:
Enabled: true

Metrics/AbcSize:
Max: 28

Metrics/ClassLength:
Max: 150

Metrics/CyclomaticComplexity:
Max: 8

Metrics/PerceivedComplexity:
Max: 10

Metrics/ParameterLists:
Max: 5
CountKeywordArgs: false

Metrics/MethodLength:
Max: 41

Metrics/ModuleLength:
Max: 180

RSpec/DescribeClass:
Enabled: false

RSpec/ExampleLength:
Enabled: false

RSpec/MessageSpies:
Enabled: false

RSpec/MessageChain:
Enabled: false

RSpec/MultipleExpectations:
Enabled: false

RSpec/MultipleMemoizedHelpers:
Max: 12

RSpec/NestedGroups:
Max: 5

Style/FrozenStringLiteralComment:
EnforcedStyle: always
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

File must end with a newline character.

The file is missing a trailing newline after line 99, which violates the coding guideline that states: "ALWAYS ensure files end with a newline character before committing/pushing".

As per coding guidelines, add a newline at the end of the file:

 Style/FrozenStringLiteralComment:
   EnforcedStyle: always
+
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Root RuboCop configuration for React on Rails monorepo
# This contains shared settings inherited by react_on_rails/ and react_on_rails_pro/
require:
- rubocop-performance
- rubocop-rspec
AllCops:
NewCops: enable
DisplayCopNames: true
SuggestExtensions: false
TargetRubyVersion: 3.0
Include:
- '**/Rakefile'
- '**/config.ru'
- 'Gemfile'
- '**/*.rb'
- '**/*.rake'
Exclude:
- '**/*.js'
- '**/node_modules/**/*'
- '**/public/**/*'
- '**/tmp/**/*'
- 'coverage/**/*'
- 'gen-examples/**/*'
- 'node_modules/**/*'
- 'spec/fixtures/**/*'
- 'spec/react_on_rails/dummy-for-generators/**/*'
- 'vendor/**/*'
Naming/FileName:
Exclude:
- '**/Gemfile'
- '**/Rakefile'
Layout/LineLength:
Max: 120
Style/StringLiterals:
EnforcedStyle: double_quotes
Style/Documentation:
Enabled: false
Style/HashEachMethods:
Enabled: true
Style/HashTransformKeys:
Enabled: true
Style/HashTransformValues:
Enabled: true
Metrics/AbcSize:
Max: 28
Metrics/ClassLength:
Max: 150
Metrics/CyclomaticComplexity:
Max: 8
Metrics/PerceivedComplexity:
Max: 10
Metrics/ParameterLists:
Max: 5
CountKeywordArgs: false
Metrics/MethodLength:
Max: 41
Metrics/ModuleLength:
Max: 180
RSpec/DescribeClass:
Enabled: false
RSpec/ExampleLength:
Enabled: false
RSpec/MessageSpies:
Enabled: false
RSpec/MessageChain:
Enabled: false
RSpec/MultipleExpectations:
Enabled: false
RSpec/MultipleMemoizedHelpers:
Max: 12
RSpec/NestedGroups:
Max: 5
Style/FrozenStringLiteralComment:
EnforcedStyle: always
# Root RuboCop configuration for React on Rails monorepo
# This contains shared settings inherited by react_on_rails/ and react_on_rails_pro/
require:
- rubocop-performance
- rubocop-rspec
AllCops:
NewCops: enable
DisplayCopNames: true
SuggestExtensions: false
TargetRubyVersion: 3.0
Include:
- '**/Rakefile'
- '**/config.ru'
- 'Gemfile'
- '**/*.rb'
- '**/*.rake'
Exclude:
- '**/*.js'
- '**/node_modules/**/*'
- '**/public/**/*'
- '**/tmp/**/*'
- 'coverage/**/*'
- 'gen-examples/**/*'
- 'node_modules/**/*'
- 'spec/fixtures/**/*'
- 'spec/react_on_rails/dummy-for-generators/**/*'
- 'vendor/**/*'
Naming/FileName:
Exclude:
- '**/Gemfile'
- '**/Rakefile'
Layout/LineLength:
Max: 120
Style/StringLiterals:
EnforcedStyle: double_quotes
Style/Documentation:
Enabled: false
Style/HashEachMethods:
Enabled: true
Style/HashTransformKeys:
Enabled: true
Style/HashTransformValues:
Enabled: true
Metrics/AbcSize:
Max: 28
Metrics/ClassLength:
Max: 150
Metrics/CyclomaticComplexity:
Max: 8
Metrics/PerceivedComplexity:
Max: 10
Metrics/ParameterLists:
Max: 5
CountKeywordArgs: false
Metrics/MethodLength:
Max: 41
Metrics/ModuleLength:
Max: 180
RSpec/DescribeClass:
Enabled: false
RSpec/ExampleLength:
Enabled: false
RSpec/MessageSpies:
Enabled: false
RSpec/MessageChain:
Enabled: false
RSpec/MultipleExpectations:
Enabled: false
RSpec/MultipleMemoizedHelpers:
Max: 12
RSpec/NestedGroups:
Max: 5
Style/FrozenStringLiteralComment:
EnforcedStyle: always
🤖 Prompt for AI Agents
In .rubocop.yml around lines 1 to 99, the file is missing a trailing newline at
the end (line 99); add a single newline character as the final byte of the file
so it ends with a newline, then save/commit the change.

- '**/*.rb'
- '**/*.rake'

EnableNewCops: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

RuboCop NewCops vs EnableNewCops settings documentation

💡 Result:

Short answer — use AllCops: NewCops. NewCops (values: enable / disable / pending) is the official RuboCop setting to control how "pending" (newly added between major releases) cops are handled; it lets you bulk-enable or bulk-disable those cops or leave them pending so RuboCop warns you and you enable/disable them individually. (docs.rubocop.org)

"EnableNewCops" is not an official RuboCop config key — you'll see it sometimes in community gems/configs or posts, but the documented/supported form is AllCops: NewCops. Put NewCops under AllCops in .rubocop.yml (or use the --enable-pending-cops / --disable-pending-cops CLI flags). (docs.rubocop.org)

If you want the authoritative docs, see:

If you want, I can show a minimal .rubocop.yml example.

Citations:


Remove or replace EnableNewCops: true — it is not an official RuboCop configuration key.

RuboCop only recognizes NewCops (with values: enable, disable, or pending) placed under AllCops. The setting EnableNewCops: true will be silently ignored by RuboCop and has no effect. The root config's NewCops: enable is the correct syntax.

🤖 Prompt for AI Agents
In react_on_rails_pro/.rubocop.yml around line 5, the setting `EnableNewCops:
true` is not a valid RuboCop key and should be removed; instead add or ensure an
`AllCops` section contains `NewCops: enable` (e.g. under `AllCops:` set
`NewCops: enable`) so RuboCop recognizes the intent to enable new cops.

gem "steep", require: false
gem "rubocop", "1.61.0", require: false
gem "rubocop-performance", "~>1.20.0", require: false
gem "rubocop-rails-omakase", "~>1.0.0", require: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if rubocop-rails-omakase is required in any RuboCop config
rg -n "rubocop-rails-omakase" --type=yaml

Repository: shakacode/react_on_rails

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Examine the .rubocop.yml file to see the require section
cat -n .rubocop.yml

Repository: shakacode/react_on_rails

Length of output: 2352


🏁 Script executed:

#!/bin/bash
# Check the Gemfile.development_dependencies file
cat -n react_on_rails/Gemfile.development_dependencies

Repository: shakacode/react_on_rails

Length of output: 2161


🏁 Script executed:

#!/bin/bash
# Check if Gemfile.development_dependencies ends with a newline
tail -c 1 react_on_rails/Gemfile.development_dependencies | od -An -tx1

Repository: shakacode/react_on_rails

Length of output: 110


🏁 Script executed:

#!/bin/bash
# Check if file ends with newline using a different method
if [ -n "$(tail -c 1 react_on_rails/Gemfile.development_dependencies)" ]; then
  echo "NO newline at end"
else
  echo "File ends with newline"
fi

Repository: shakacode/react_on_rails

Length of output: 90


Add rubocop-rails-omakase to the require: section in .rubocop.yml.

The gem is added to Gemfile.development_dependencies but is not required in .rubocop.yml. Only rubocop-performance and rubocop-rspec are currently required. Without explicit configuration, the cops from rubocop-rails-omakase won't be active and its rules won't be enforced.

🤖 Prompt for AI Agents
In react_on_rails/Gemfile.development_dependencies around line 40, the gem
"rubocop-rails-omakase" was added but its cops are not enabled in .rubocop.yml;
update .rubocop.yml to add "rubocop-rails-omakase" to the require: list
alongside "rubocop-performance" and "rubocop-rspec" so the gem's cops are loaded
(edit the top-level require section to include the new gem name).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants