-
-
Notifications
You must be signed in to change notification settings - Fork 638
Fix /run-skipped-ci command to only run minimum dependency tests #2005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This adds a `/run-full-ci` command that can be used in PR comments to run the complete CI suite including tests normally skipped on PRs. **New workflows:** - `run-skipped-ci.yml` - Triggers on `/run-full-ci` comment, dispatches all CI workflows - `pr-welcome-comment.yml` - Auto-comments on new PRs explaining the command **Features:** - Runs full test matrix (all Ruby/Node versions) - Runs example generator tests - Runs Pro package integration and unit tests - Posts acknowledgment comment with workflow links - Adds rocket reaction to trigger comment **Documentation:** - Added `.github/README.md` with comprehensive docs - Explains why subset CI exists (fast feedback) - Documents testing limitations (must be on master branch) - Lists all available workflows and their purposes This improves the PR workflow by letting contributors easily run full CI on-demand without waiting for merge to master. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This addresses critical security and reliability issues identified in the initial implementation of the /run-full-ci comment trigger. **Security Fixes:** - Add permission check to restrict command to users with write access - Prevents resource abuse from external contributors - Posts informative message to unauthorized users **Reliability Improvements:** - Consolidate workflow triggers with better error handling - Post detailed results comment listing succeeded/failed workflows - Fail job if any workflows fail to trigger - Add concurrency controls to prevent duplicate runs per PR - Improve comment matching to require command at line start **Workflow Updates:** - Update main.yml, examples.yml to run minimum deps on workflow_dispatch - Update Pro workflows to honor workflow_dispatch events - Skip welcome comment for bot PRs (dependabot, renovate) **Documentation:** - Document security controls and access restrictions - Document concurrency protection - Explain workflow_dispatch behavior These changes ensure the feature is secure, reliable, and provides clear feedback when workflows fail to trigger. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The previous approach using matrix.exclude with matrix context expressions caused actionlint errors because the matrix context is not available in the exclude section. **Fix:** - Move conditional logic to job-level if statements - Check both event type and dependency level in single expression - For regular PRs: only run latest versions - For master/workflow_dispatch: run all versions including minimum This properly filters matrix jobs without using unsupported contexts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Changes the PR comment command to match the workflow filename for consistency. **Changes:** - Update workflow trigger to listen for `/run-skipped-ci` - Update welcome comment to show `/run-skipped-ci` - Update README documentation to reference `/run-skipped-ci` - Update workflow filename reference in docs The command name now clearly indicates it runs the normally-skipped CI jobs (minimum versions, examples, Pro tests). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The previous approach of using matrix context in job-level if statements caused actionlint errors. Fixed by using environment variables and step-level conditionals instead. **Solution:** - Set SKIP_MINIMUM env var at job level using matrix context - Add conditional `if: env.SKIP_MINIMUM != 'true'` to all steps - First step exits early with helpful message on regular PRs - Minimum versions only run on master or workflow_dispatch This approach is actionlint-compliant and properly filters matrix jobs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Remove duplicate if conditions in examples.yml and main.yml that caused knip YAML parsing errors - Replace SKIP_MINIMUM_ON_PR env var pattern with cleaner matrix-level exclude approach - Add workflow verification in run-skipped-ci.yml to confirm workflows actually start - Improve error handling with detailed status reporting showing verified/pending/failed workflows - Fix actionlint validation errors This eliminates inefficient step-level conditionals and ensures workflows are properly filtered at the job level, making them more efficient and easier to maintain. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The /run-skipped-ci command was triggering ALL workflows, which caused detect-changes to skip them (since they run on the PR branch with no Pro file changes). Now it correctly: 1. Only triggers workflows with minimum dependency matrix - main.yml: Ruby 3.2, Node 20 - examples.yml: Ruby 3.2 2. Skips workflows that already run on all PRs - pro-integration-tests.yml (always runs) - pro-package-tests.yml (always runs) 3. Uses workflow_dispatch inputs to control matrix - Added run_minimum_tests input to main.yml and examples.yml - When true, excludes latest matrix (3.4/22) - When false, excludes minimum matrix on PRs (existing behavior) 4. Updates PR comment to show what ran and what was skipped - Clear explanation of which tests are running - Note about why Pro tests are skipped This fixes the issue where detect-changes would stop the workflow because it detected no Pro file changes on the PR branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 6 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (7)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
|
Closing in favor of PR with clean branch (no merge conflicts) |
Summary
Fixes the
/run-skipped-cicommand which was broken because:detect-changesjob would run on the PR branch and see no Pro file changesChanges
1. Run Only Minimum Dependency Tests
The command now only triggers workflows with minimum dependency configurations that are normally skipped on PRs:
It skips Pro workflows because they already run on all PRs (no need to re-run them).
2. Added Workflow Inputs
Added
run_minimum_testsinput tomain.ymlandexamples.yml:3. Smart Matrix Exclusion
Updated matrix exclusion logic to respect the input:
run_minimum_tests=true: Exclude latest matrix (run only minimum)run_minimum_tests=false: Exclude minimum on PRs (existing behavior)Uses
fromJSON()to avoid the empty string issue with conditional excludes:4. Improved PR Comments
The PR comment now clearly shows:
Example:
Testing
/run-skipped-cion a PRRelated Issues
Closes #1999 (uses better matrix exclusion approach)
🤖 Generated with Claude Code
This change is