Skip to content

Conversation

@justin808
Copy link
Member

Summary

  • Restores bin/shakapacker-config at project root (was moved by PR Fix development for spec/dummy #841, breaking integration tests)
  • Fixes quote style in spec/dummy/bin/shakapacker-config to match local .prettierrc.yaml (single quotes)

This fixes two pre-existing CI failures that appear on the main branch:

  1. Node based checks (Linting) - Prettier failing on spec/dummy/bin/shakapacker-config
  2. Testing (ubuntu-latest, 20.x/22.x) - Integration tests can't find bin/shakapacker-config

Test plan

  • Verified locally: yarn prettier --check . passes
  • Verified locally: yarn lint passes
  • Verified locally: yarn test passes (319 tests)
  • CI should pass with these changes

🤖 Generated with Claude Code

PR #841 moved bin/shakapacker-config to spec/dummy/bin/shakapacker-config,
which broke the integration tests that expect it at the project root.
Additionally, the spec/dummy version had double quotes that conflict with
the local .prettierrc.yaml (singleQuote: true).

This commit:
- Restores bin/shakapacker-config at the project root for integration tests
- Fixes quote style in spec/dummy/bin/shakapacker-config to use single quotes

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Jan 3, 2026

Warning

Rate limit exceeded

@justin808 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 53 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 2d74b61 and 5929ab2.

📒 Files selected for processing (2)
  • bin/shakapacker-config
  • spec/dummy/bin/shakapacker-config
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jg/fix-ci-prettier-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.

@greptile-apps
Copy link

greptile-apps bot commented Jan 3, 2026

Greptile Summary

This PR fixes two pre-existing CI failures introduced by PR #841: restores bin/shakapacker-config at project root (needed by integration tests) and changes double quotes to single quotes in spec/dummy/bin/shakapacker-config to match prettier config.

  • Restored bin/shakapacker-config at project root - integration tests expect this file location
  • Fixed quote style in spec/dummy/bin/shakapacker-config from double to single quotes per singleQuote: true in .prettierrc.yaml

Confidence Score: 5/5

  • This PR is safe to merge - it's a straightforward fix for known CI failures
  • The changes are minimal and surgical: restoring a file that was incorrectly moved and fixing a simple quote style issue. Both changes directly address documented CI failures with no logic changes to the actual code
  • No files require special attention

Important Files Changed

Filename Overview
bin/shakapacker-config Restored file to fix integration tests that expect it at project root
spec/dummy/bin/shakapacker-config Fixed quote style to match prettier config (single quotes)

Sequence Diagram

sequenceDiagram
    participant CI as CI Pipeline
    participant Prettier as Prettier Linter
    participant Tests as Integration Tests
    participant Root as bin/shakapacker-config
    participant Dummy as spec/dummy/bin/shakapacker-config
    
    Note over CI,Dummy: Before PR #858 (after PR #841)
    CI->>Prettier: Run prettier --check
    Prettier->>Dummy: Check quote style
    Dummy-->>Prettier: ❌ Double quotes found (expects single)
    Prettier-->>CI: ❌ Linting failed
    
    CI->>Tests: Run integration tests
    Tests->>Root: Look for bin/shakapacker-config
    Root-->>Tests: ❌ File not found (moved to spec/dummy)
    Tests-->>CI: ❌ Tests failed
    
    Note over CI,Dummy: After PR #858
    CI->>Prettier: Run prettier --check
    Prettier->>Dummy: Check quote style
    Dummy-->>Prettier: ✓ Single quotes match config
    Prettier-->>CI: ✓ Linting passed
    
    CI->>Tests: Run integration tests
    Tests->>Root: Look for bin/shakapacker-config
    Root-->>Tests: ✓ File found at root
    Tests-->>CI: ✓ Tests passed
Loading

@claude
Copy link

claude bot commented Jan 3, 2026

Code Review

Summary

This PR successfully fixes two pre-existing CI failures by:

  1. Restoring bin/shakapacker-config at the project root (required by integration tests)
  2. Fixing quote style in spec/dummy/bin/shakapacker-config to match the dummy app's Prettier config

Code Quality ✅

Strengths:

  • Minimal, focused changes: Only touches what's necessary to fix the CI failures
  • Correct prettier configuration: The quote style change is appropriate - spec/dummy/.prettierrc.yaml has singleQuote: true, while the root project uses double quotes (prettier default)
  • File restoration: The bin/shakapacker-config file is needed by integration tests at test/configExporter/integration.test.js:15 which references ../../bin/shakapacker-config
  • Proper executable permissions: The new file has correct #!/usr/bin/env node shebang and executable bit

Potential Issues ⚠️

Minor concern - Inconsistent quote styles between root and dummy:
The root bin/shakapacker-config uses double quotes (line 4) while spec/dummy/bin/shakapacker-config uses single quotes (line 4). This is technically correct per each directory's prettier config, but creates inconsistency in identical files.

Recommendation: Consider whether spec/dummy should inherit the root prettier config, or if this intentional difference serves a testing purpose. If it's just historical, standardizing could reduce confusion.

Test Coverage ✅

Good:

  • The PR mentions running yarn prettier --check ., yarn lint, and yarn test locally
  • The restored file is directly referenced by integration tests
  • No new functionality added, so existing test coverage is sufficient

Suggestion: Consider adding a simple test to verify bin/shakapacker-config exists and is executable, to prevent future regressions from accidentally removing it again.

Performance Considerations ✅

No performance impact - these are simple script files that delegate to the TypeScript module.

Security Considerations ✅

  • The bin/shakapacker-config file is a minimal shim that delegates to the TypeScript module
  • Uses process.argv.slice(2) correctly to pass arguments
  • Proper error handling with exit codes
  • No external input validation needed at this level (handled in the TypeScript module)
  • No security concerns identified

Best Practices ✅

  • Follows project conventions: Both files end with trailing newlines as required by CLAUDE.md
  • Proper git workflow: PR created for review before merge
  • Clear PR description: Explains what was broken and how it's fixed

Additional Notes

Why this broke:
PR #841 appears to have removed or moved the root bin/shakapacker-config, breaking integration tests that expect it at that location.

File structure rationale:

  • bin/shakapacker-config: Used by integration tests and likely by users who install the gem
  • spec/dummy/bin/shakapacker-config: Used within the dummy Rails app for testing

Both are necessary and serve different purposes.

Conclusion ✅

Recommendation: APPROVE

This is a straightforward, well-executed fix for CI failures. The changes are minimal, correct, and restore necessary functionality. The only minor point is the quote style inconsistency, which is more of an observation than a blocker.

Great work identifying and fixing these issues! 🎯

@justin808 justin808 merged commit 86de096 into main Jan 3, 2026
7 checks passed
@justin808 justin808 deleted the jg/fix-ci-prettier-config branch January 3, 2026 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants