Skip to content

[CI] Move ci path filtering funcs into separate file#3256

Merged
HereThereBeDragons merged 2 commits into
mainfrom
users/lpromber/move_ci_path_filtering
Feb 5, 2026
Merged

[CI] Move ci path filtering funcs into separate file#3256
HereThereBeDragons merged 2 commits into
mainfrom
users/lpromber/move_ci_path_filtering

Conversation

@HereThereBeDragons
Copy link
Copy Markdown
Contributor

@HereThereBeDragons HereThereBeDragons commented Feb 4, 2026

Move all functions related to ci path filtering and determining based on this if the CI should be run or not into a separate file configure_ci_path_filters.py.

Aside from adjusting description, also gives better names to the following functions:

  • get_modified_paths -> get_git_modified_paths
  • get_therock_submodule_paths -> get_git_submodule_paths
  • should_ci_run_given_modified_paths -> is_ci_run_required

Part of CI weekly progress (big picture #1732 ) so that the new and old CI configurators can share those functions.

Comment on lines +62 to +66
from ci_path_filters import (
get_git_modified_paths_since,
get_git_submodule_paths,
is_ci_run_required,
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jayhawk-commits has work in progress at #3181 that I believe will allow rocm-libraries and rocm-systems to plug in their own logic here with code in e.g. https://github.com/ROCm/rocm-libraries/blob/develop/.github/scripts/therock_configure_ci.py

I think this refactoring is a net improvement but let's check if it aligns with those plans or if a slightly different approach would be more composable

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactoring in this PR would be useful for the pending work in #3188 where I am moving some shared configure functions useful to external repos into a shared configure module, and the super-repo specific functionality in another configure module. The work here overlaps with some of the functionality of those modules.

As far as #3181 goes, I think no changes are required for now. The changes would be in #3188 where I would make functions like _is_path_skippable work for external repos, by pulling in the skippable path list from the other repo through the work in #3181.

I think #3181 and #3256 can land simultaneously without issues. I'll modify #3188 after both land.

Comment thread build_tools/github_actions/configure_ci_path_filters.py
Comment thread build_tools/github_actions/tests/configure_ci_test.py Outdated
Comment thread build_tools/github_actions/configure_ci_path_filters.py
Copy link
Copy Markdown
Member

@ScottTodd ScottTodd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % the comments about file / test naming

Copy link
Copy Markdown
Contributor

@geomin12 geomin12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for splitting this up! i would agree with Joseph + Scott in regards to adding configure_ci_path_filters.py as the name of the file

Copy link
Copy Markdown
Member

@marbre marbre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once Scott's naming comments are addressed. Eventually consider changing the one function name as well but beside that good to land.

Comment thread build_tools/github_actions/ci_path_filters.py Outdated
@HereThereBeDragons HereThereBeDragons force-pushed the users/lpromber/move_ci_path_filtering branch from 7771d26 to 1dfc28a Compare February 5, 2026 11:35
@HereThereBeDragons HereThereBeDragons merged commit 553a800 into main Feb 5, 2026
77 of 91 checks passed
@HereThereBeDragons HereThereBeDragons deleted the users/lpromber/move_ci_path_filtering branch February 5, 2026 19:43
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Feb 5, 2026
radhaksri added a commit that referenced this pull request Feb 13, 2026
Cherry-picked commit from ROCm/rocm-systems#3256:
- rocr: IPC: Manage IPC socket thread lifetime
radhaksri added a commit that referenced this pull request Feb 26, 2026
…, #3256, #3240, #3208

Cherry-picked commits:
- PR #3256: 3 commits for IPC socket thread management and error handling
- PR #3240: 1 commit for ASan use-after-free fix in system region cleanup
- PRs #3407, #3208: Already included in previous cherry-picks

These changes improve ASAN compatibility and fix memory management issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants