Skip to content

ci: apply change detection to pull_request_target events - #1104

Merged
slin1237 merged 1 commit into
smg-project:mainfrom
ai-jz:ci/change-detection-for-fork-prs
Apr 12, 2026
Merged

slin1237 merged 1 commit into
smg-project:mainfrom
ai-jz:ci/change-detection-for-fork-prs

Conversation

@ai-jz

@ai-jz ai-jz commented Apr 12, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

PR #1095 (commit 381c5f6) implemented change detection for ci-approved fork PRs by adding pull_request_target to the detect-changes job. However, the 7 downstream e2e job conditions only check filter outputs for pull_request events. This PR extends them to also check for pull_request_target, so ci-approved fork PRs skip unrelated e2e categories — same as direct PRs.

Changes

Added && github.event_name != 'pull_request_target' to all 7 e2e job conditions so that path-based change detection applies equally to ci-approved fork PRs.

Context

PR #1095 (commit 381c5f6) implemented change detection for ci-approved fork PRs by adding pull_request_target to the workflow triggers, updating detect-changes to run for both event types, and adding ci-gate for label checking. This PR extends the same approach to the 7 downstream e2e job conditions.

cc @slin1237 — this extends your work in #1095.

Test plan

  • pre-commit passes
  • YAML syntax valid (checked by check yaml hook)
  • Verify on a real ci-approved fork PR that touches only one category's paths

Summary by CodeRabbit

  • Chores
    • Refined GitHub Actions workflow conditions to improve pull request event handling and ensure proper CI/CD test execution across multiple workflows.

The detect-changes job already runs for pull_request_target (fork PRs),
but the downstream e2e job conditions only checked for pull_request,
causing all engines to run unconditionally on fork PRs regardless of
which paths were modified. This wastes GPU time when a fork PR only
touches one engine's code.

Add pull_request_target to the event_name check in all 7 e2e job
conditions so path-based filtering applies equally to fork PRs.

Signed-off-by: ai-jz <ai-jz@users.noreply.github.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@github-actions github-actions Bot added the ci CI/CD configuration changes label Apr 12, 2026
@coderabbitai

coderabbitai Bot commented Apr 12, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1a6f14b2-0793-4829-9e7a-a4550472e127

📥 Commits

Reviewing files that changed from the base of the PR and between ae14a9c and ec193ec.

📒 Files selected for processing (1)
  • .github/workflows/pr-test-rust.yml

📝 Walkthrough

Walkthrough

Updated job-level conditional statements in .github/workflows/pr-test-rust.yml to exclude pull_request_target events from E2E, vendor, and go-bindings jobs. These jobs now follow the same event-filtering logic as the existing detect-changes gating by adding && github.event_name != 'pull_request_target' to their if: conditions.

Changes

Cohort / File(s) Summary
CI Workflow Event Filtering
.github/workflows/pr-test-rust.yml
Added pull_request_target event exclusion to job-level if: conditions for E2E, vendor, and go-bindings jobs to align with detect-changes gating logic.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

Suggested labels

ci

Suggested reviewers

  • CatherineSue
  • key4ng
  • XinyueZhang369

Poem

🐰 A rabbit hops through workflows bright,
Excluding targets left and right,
Jobs align with gating's might,
CI flows true—pure delight! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: applying change detection (path-based filtering) to pull_request_target events in CI workflows.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@slin1237
slin1237 merged commit b776668 into smg-project:main Apr 12, 2026
31 of 32 checks passed
slin1237 added a commit that referenced this pull request Apr 17, 2026
This reverts commit b776668.

Reverting as part of rolling back the ci-approved fork PR trigger
added in #1095. Once the pull_request_target workflow trigger is
removed, these e2e job conditions referring to
`github.event_name != 'pull_request_target'` become dead code and
would never evaluate, so they are reverted together.

Refs: #1095, #1104
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI/CD configuration changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants