Skip to content

Conversation

@justin808
Copy link
Member

@justin808 justin808 commented Nov 10, 2025

Summary

Add RBS type signature validation to the CI pipeline to catch type signature regressions automatically before merge.

Changes

  • Added RBS validation step to .github/workflows/lint-js-and-ruby.yml
  • Step runs bundle exec rbs -I sig validate after RuboCop linting
  • Uses the same validation command as the existing rake rbs:validate task

Benefits

  • Prevents type signature drift
  • Catches type errors before merge
  • Ensures signatures stay in sync with implementation changes
  • Provides confidence that type signatures remain valid

Testing

  • Verified RBS validation passes locally: bundle exec rbs -I sig validate
  • All existing linting checks pass

Fixes #1952

🤖 Generated with Claude Code


This change is Reviewable

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 10, 2025

Warning

Rate limit exceeded

@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 34 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 40b79b8 and 134dc39.

📒 Files selected for processing (1)
  • .github/workflows/lint-js-and-ruby.yml (1 hunks)
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch justin808/add-rbs-ci-validation

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.

@justin808
Copy link
Member Author

@claude review this PR

@claude
Copy link

claude bot commented Nov 12, 2025

Claude finished @justin808's task —— View job


PR Review Complete

Task Checklist:

  • Read the modified workflow file
  • Examine RBS validation implementation
  • Check related RBS configuration files
  • Provide comprehensive review feedback

Review Summary

This PR adds RBS type signature validation to the CI pipeline. The implementation is clean and well-placed, but there are a few suggestions for improvement.

Findings

Positives

  1. Correct placement: The RBS validation step is placed right after RuboCop linting (.github/workflows/lint-js-and-ruby.yml:92-93), which is logical since both are Ruby code quality checks.

  2. Command consistency: Uses the exact same command as the existing rake rbs:validate task (bundle exec rbs -I sig validate from rakelib/rbs.rake:13).

  3. Proper dependency: RBS gem is already present in Gemfile.development_dependencies:36 as a development dependency.

  4. Good commit message: Clear, descriptive, and includes the issue reference.

  5. Existing RBS infrastructure: The project has 10 RBS signature files in sig/ directory with a documented validation process in sig/README.md.

Suggestions

  1. Missing Pro package validation: The Pro package has its own linting workflow (.github/workflows/pro-lint.yml) that also runs RuboCop (line 124). If the Pro package has RBS signatures, it should also include this validation step for consistency.

    Recommendation: Check if react_on_rails_pro/ has RBS signatures. If yes, add the same validation step to pro-lint.yml after line 124:

    - name: Validate RBS type signatures
      run: bundle exec rbs -I sig validate
  2. Step naming consistency: The step name "Validate RBS type signatures" is clear and descriptive, matching the style of other steps in the workflow.

  3. No failure handling needed: Since this is a validation step, the default behavior (fail the workflow on non-zero exit code) is correct. The rake task already handles this properly.

Potential Issues

None identified. The implementation is correct and should work as expected.

Additional Context

  • The PR description accurately describes the changes and benefits
  • Fixes issue Add RBS validation to CI pipeline #1952 as stated
  • The validation will run on every PR and push to master that affects non-documentation files
  • CI will fail fast if RBS signatures become invalid, preventing type signature drift

Verdict

LGTM with one optional suggestion: Consider adding RBS validation to the Pro package workflow if it has type signatures.

The core change is solid and ready to merge. The Pro package validation is optional depending on whether the Pro package uses RBS.


@justin808 justin808 closed this Nov 13, 2025
@justin808 justin808 force-pushed the justin808/add-rbs-ci-validation branch from 134dc39 to c1ae439 Compare November 13, 2025 03:28
@justin808 justin808 deleted the justin808/add-rbs-ci-validation branch November 13, 2025 03:28
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.

Add RBS validation to CI pipeline

2 participants