Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
📝 WalkthroughWalkthroughThe pull request updates the GitHub Actions workflow to respond to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 Tip You can customize the tone of the review comments and chat replies.Configure the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-test.yml:
- Around line 73-78: The workflow currently sets authorized=false for draft PRs
which downstream logic treats as an authorization failure; instead stop
assigning authorized here and emit a distinct skip signal so downstream can
treat it as a deliberate skip. Replace the echo "authorized=false" in the draft
branch with an output like "skip_reason=draft" (or "skipped=true") and keep
authorized unset (or explicitly leave it true elsewhere only when real
authorization is granted) so downstream summary logic can detect
skip_reason/skipped and exit cleanly without treating it as an auth failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 490c55fa-147f-40a6-9d1f-63bc7884cf0c
📒 Files selected for processing (1)
.github/workflows/pr-test.yml
| # Skip draft PRs to save CI capacity | ||
| if [[ "${{ github.event.pull_request.draft }}" == "true" ]]; then | ||
| echo "authorized=false" >> "$GITHUB_OUTPUT" | ||
| echo "Draft PR, skipping CI" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
Draft skip currently routes into “unauthorized” failure path.
Line 75 sets authorized=false for drafts, but downstream summary logic handles this as “pending authorization” and exits with failure. That makes draft PRs fail instead of being cleanly skipped.
💡 Proposed fix (distinguish skip reason from authorization)
gate:
name: Permission Check
runs-on: ubuntu-latest
outputs:
authorized: ${{ steps.check.outputs.authorized }}
+ skip_reason: ${{ steps.check.outputs.skip_reason }}
@@
if [[ "${{ github.event_name }}" != "pull_request" ]]; then
echo "authorized=true" >> "$GITHUB_OUTPUT"
+ echo "skip_reason=none" >> "$GITHUB_OUTPUT"
echo "Not a PR, authorized"
exit 0
fi
# Skip draft PRs to save CI capacity
if [[ "${{ github.event.pull_request.draft }}" == "true" ]]; then
echo "authorized=false" >> "$GITHUB_OUTPUT"
+ echo "skip_reason=draft" >> "$GITHUB_OUTPUT"
echo "Draft PR, skipping CI"
exit 0
fi
@@
if [[ "${{ contains(github.event.pull_request.labels.*.name, 'run-ci') }}" == "true" ]]; then
echo "authorized=true" >> "$GITHUB_OUTPUT"
+ echo "skip_reason=none" >> "$GITHUB_OUTPUT"
echo "PR has run-ci label, authorized"
exit 0
fi
@@
echo "authorized=true" >> "$GITHUB_OUTPUT"
+ echo "skip_reason=none" >> "$GITHUB_OUTPUT"
@@
echo "authorized=false" >> "$GITHUB_OUTPUT"
+ echo "skip_reason=unauthorized" >> "$GITHUB_OUTPUT"
@@
if echo "$MEMBERS" | grep -qx "$AUTHOR"; then
echo "authorized=true" >> "$GITHUB_OUTPUT"
+ echo "skip_reason=none" >> "$GITHUB_OUTPUT"
echo "$AUTHOR is a member of $TEAM, authorized"
else
echo "authorized=false" >> "$GITHUB_OUTPUT"
+ echo "skip_reason=unauthorized" >> "$GITHUB_OUTPUT"
echo "$AUTHOR is not a member of $TEAM, not authorized"
fi test-results-summary:
@@
+ - name: Report draft PR status
+ if: needs.gate.outputs.skip_reason == 'draft'
+ run: |
+ echo "## Test Results Summary" >> $GITHUB_STEP_SUMMARY
+ echo "CI skipped (draft PR)" >> $GITHUB_STEP_SUMMARY
+ echo "::notice::Draft PR — CI skipped"
+ exit 0
+
- name: Check Authorization
- if: needs.gate.outputs.authorized != 'true'
+ if: needs.gate.outputs.authorized != 'true' && needs.gate.outputs.skip_reason != 'draft'
uses: actions/checkout@v4
@@
- name: Check skip patterns for unauthorized PRs
- if: needs.gate.outputs.authorized != 'true'
+ if: needs.gate.outputs.authorized != 'true' && needs.gate.outputs.skip_reason != 'draft'
id: skip-check
@@
- name: Report unauthorized PR status
- if: needs.gate.outputs.authorized != 'true'
+ if: needs.gate.outputs.authorized != 'true' && needs.gate.outputs.skip_reason != 'draft'
run: |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-test.yml around lines 73 - 78, The workflow currently
sets authorized=false for draft PRs which downstream logic treats as an
authorization failure; instead stop assigning authorized here and emit a
distinct skip signal so downstream can treat it as a deliberate skip. Replace
the echo "authorized=false" in the draft branch with an output like
"skip_reason=draft" (or "skipped=true") and keep authorized unset (or explicitly
leave it true elsewhere only when real authorization is granted) so downstream
summary logic can detect skip_reason/skipped and exit cleanly without treating
it as an auth failure.
📌 Description
Saves testing capacity not running per-pr testing on draft PRs
🔍 Related Issues
🚀 Pull Request Checklist
Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete.
✅ Pre-commit Checks
pre-commitby runningpip install pre-commit(or used your preferred method).pre-commit install.pre-commit run --all-filesand fixed any reported issues.🧪 Tests
unittest, etc.).Reviewer Notes
Summary by CodeRabbit