Repository navigation
[ci] Trigger full project matrix on ci-env/action.yml changes - #8249
Conversation
The TheRock ref is centralized in .github/actions/ci-env/action.yml (the single source of truth from #7262), but therock_configure_ci.py only treated .github/workflows/ and .github/scripts/ changes as CI-relevant. As a result, a ref bump that edits only ci-env/action.yml did not trigger the full build/test matrix that validates it. Add .github/actions/ci-env/ as a CI-relevant path so centralized ref bumps exercise CI.
github-actions
Bot
added
github actions
project: none
There was a problem hiding this comment.
Pull request overview
Updates CI-change detection so that edits to the centralized CI environment composite action (.github/actions/ci-env/action.yml) are treated as CI-relevant, ensuring TheRock ref-only bumps trigger the full project build/test matrix again.
Changes:
- Teach
is_path_workflow_file_related_to_ci()to consider.github/actions/ci-env/action.ymlCI-relevant (while preserving existing workflow/script behavior). - Add a unit test covering the new
ci-env/action.ymlbehavior and a negative test for an unrelated action path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| .github/scripts/therock_configure_ci.py | Extend CI-related path matching to include .github/actions/ci-env/action.yml. |
| .github/scripts/tests/therock_configure_ci_test.py | Add assertions verifying ci-env/action.yml is CI-relevant and unrelated actions are not. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
geomin12
left a comment
geomin12
left a comment
There was a problem hiding this comment.
lgtm but pre-commit is failing
Collapse the redundant parentheses around pr_labels_json into a single line so the pre-commit black hook passes. Co-authored-by: Cursor <cursoragent@cursor.com>
davidd-amd
left a comment
davidd-amd
left a comment
There was a problem hiding this comment.
This looks ok to me just curious why we have the two different action.yml files.
|
@copilot resolve the merge conflicts in this pull request |
Head branch was pushed to by a user without write access
Resolved by merging |
Copilot's merge commit left only one blank line before is_path_workflow_file_related_to_ci; black requires two. Add the missing blank line so the pre-commit black hook passes. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@tony-davis ROCm/TheRock#5620 merged today — the bot now creates bump PRs that only touch |
Per review feedback, the test-only is_path_workflow_file_related_to_ci duplicated check_for_workflow_file_related_to_ci. Drop it and have the test exercise the production function directly.
…onfigure-ci-trigger-ci-env
Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
The TheRock commit pin is centralized in
.github/actions/ci-env/action.yml(the single source of truth introduced by #7262). Buttherock_configure_ci.pyonly treated.github/workflows/and.github/scripts/changes as CI-relevant, so a ref bump that edits onlyci-env/action.ymlwasn't detected as CI-relevant — meaning it didn't trigger the full build/test matrix that validates the new ref (only the basic checks ran).This teaches
is_path_workflow_file_related_to_cito also treat.github/actions/ci-env/action.ymlas CI-relevant, so centralized TheRock ref bumps exercise the full project matrix again.Changes
.github/scripts/therock_configure_ci.py: addGITHUB_ACTIONS_CI_PATTERNS = ["ci-env/action.yml"]and a.github/actions/clause tois_path_workflow_file_related_to_ci. (The.github/workflows/+.github/scripts/checks are refactored into a single prefix loop — behaviorally identical.).github/scripts/tests/therock_configure_ci_test.py: extendtest_is_path_workflow_file_related_to_cito assertci-env/action.ymlis CI-relevant and that an unrelated action path (setup-rocm-linux/action.yml) is not.Background
This is the one still-needed piece carved out of the now-closed #8014 (a TheRock ref bump that was overcome by events once #8196 bumped
developdirectly). The bump portion of #8014 is obsolete, but this trigger fix is not yet ondevelop.Test plan
is_path_workflow_file_related_to_ciunit test updated (syntax-checked locally; full suite runs in CI).