Fix CI failures on master and add workflow_dispatch to all workflows #1928
Conversation
This commit addresses two critical CI issues:
1. **Fix generator validation failing in CI**:
- The rake task that generates example apps was calling `rails generate`
as a shell command, which spawned a new process without the
REACT_ON_RAILS_SKIP_VALIDATION environment variable set
- Modified shakapacker_examples.rake to prefix generator commands with
the ENV variable so validation is skipped during npm package installation
- This resolves the "No React on Rails npm package is installed" error
that was breaking the examples workflow on master
2. **Add workflow_dispatch to all CI workflows**:
- Added workflow_dispatch trigger to enable manual workflow runs on any
branch via GitHub Actions UI
- This makes CI testing more flexible and allows developers to test
changes on feature branches without needing to open a PR
- Updated workflows: main.yml, lint-js-and-ruby.yml,
rspec-package-specs.yml, package-js-tests.yml, pro-integration-tests.yml,
pro-package-tests.yml, pro-lint.yml
The generator validation fix complements the existing skip logic added in
PR #1923 which only handled direct generator invocations, not shell command
invocations.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
WalkthroughThe pull request adds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2025-04-26T21:55:55.874ZApplied to files:
📚 Learning: 2025-02-18T13:08:01.477ZApplied to files:
📚 Learning: 2025-02-12T16:38:06.537ZApplied to files:
⏰ 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). (2)
🔇 Additional comments (8)
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 |
Code ReviewI've reviewed this PR and here's my comprehensive feedback: Overall Assessment✅ Approve with minor suggestions This PR addresses two distinct but valuable improvements to the CI infrastructure. The changes are well-implemented and align with the repository's coding standards. Part 1: Fix Generator Validation in CICode Quality & Best Practices ✅Strengths:
Implementation (rakelib/shakapacker_examples.rake:40-44): # Set ENV variable to skip validation during generator run since npm package isn't installed yet
generator_commands = example_type.generator_shell_commands.map do |cmd|
"REACT_ON_RAILS_SKIP_VALIDATION=true \#{cmd}"
end
sh_in_dir(example_type.dir, generator_commands)Potential Issues
|
Added comprehensive comment explaining why we prefix shell commands with the ENV variable rather than setting it in the Ruby process: 1. Each shell command spawns a new process with its own environment 2. ENV variable is automatically scoped to each command (no cleanup needed) 3. Simpler and safer than the generator approach which requires explicit ENV cleanup This addresses potential confusion about ENV variable scope differences between the rake task approach and the generator approach in lib/generators/react_on_rails/install_generator.rb. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Code Review - PR #1928Overall AssessmentThis PR makes two distinct improvements: fixing a critical CI validation issue and adding workflow_dispatch to CI workflows. The changes are well-documented and follow the project's coding standards. Part 1: Fix Generator Validation Issue ✅File: rakelib/shakapacker_examples.rake Strengths
Code QualityThe implementation is clean and maintainable. The use of string interpolation to prefix each command ensures the ENV variable is properly scoped to each shell process. Potential Considerations
Part 2: Add workflow_dispatch ✅Files: All .github/workflows/*.yml files Strengths
Security Considerations✅ No security concerns identified
Performance Considerations✅ Neutral to slightly positive impact
Test CoverageCurrent State: Adequate
CLAUDE.md Compliance Check✅ Fully compliant with repository guidelines
Recommendation✅ APPROVE - Ready to merge This PR:
The changes are low-risk, high-value improvements. Once CI passes, this can be merged with confidence. Great work on identifying and fixing these issues! 🎉 Review generated using project standards from CLAUDE.md |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
This PR addresses two critical CI issues:
Changes
1. Fix Generator Validation in CI
Problem: The
shakapacker_examples.raketask was callingrails generateas a shell command, which spawned a new process without theREACT_ON_RAILS_SKIP_VALIDATIONenvironment variable set. This caused validation to run before the npm package was installed, resulting in CI failures.Solution: Modified the rake task to prefix generator commands with
REACT_ON_RAILS_SKIP_VALIDATION=true, ensuring validation is skipped during the npm package installation phase.File changed:
rakelib/shakapacker_examples.rakeThis complements the existing validation skip logic added in PR #1923, which handled direct generator invocations but not shell command invocations.
2. Add workflow_dispatch to All CI Workflows
Benefit: Developers can now manually trigger any CI workflow on any branch from the GitHub Actions UI, making it easier to:
Files changed:
.github/workflows/main.yml.github/workflows/lint-js-and-ruby.yml.github/workflows/rspec-package-specs.yml.github/workflows/package-js-tests.yml.github/workflows/pro-integration-tests.yml.github/workflows/pro-package-tests.yml.github/workflows/pro-lint.ymlTest Plan
Related Issues
🤖 Generated with Claude Code
This change is
Summary by CodeRabbit