support file-groups and improve review requests for maintainer file changes #98884
Conversation
nashif
commented
Nov 5, 2025
nashif
commented
- scripts: get_maintainer: support file groups
- scripts: set_assignee.py: Support file groups
- MAINTAINERS file: add documentation for file groups
- MAINTAINERS file: add description for tests
- ci: assigner: merge maintainer check into assigner workflow
- scripts: move set_assignee.py into scripts/ci
- scripts: set_assignee: request review from maintainers of changed areas
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
zephyrbot
requested review from
MaureenHelm,
fabiobaltieri,
kartben,
keith-zephyr,
pdgendt and
stephanosio
nashif
force-pushed
the
topic/maintainer/file-groups
branch
from
6d0a158 to
4e7b76f
Compare
nashif
force-pushed
the
topic/maintainer/file-groups
branch
from
4e7b76f to
9ea2f2f
Compare
| # different parts of the area. | ||
| # Each group should have the following structure: | ||
| # - name: <group name> | ||
| # collaborators: |
There was a problem hiding this comment.
these collaborators are added in addition to the area collaborators, right? Could maybe be mentioned.
|
copy paste comment from discord: I still sort of like my idea i described before of like the file groups only pulling from the top level area's files as a scope , otherwise you have to probably retype the same glob/regex patterns for every single file group in an area with a slight modification 
now for example if i were to make like an "NXP CAN Drivers" file group, it would basically be most of those patterns again spelled out but with an extra /can/ in them somewhere, and multiply this by like every file group |
nashif
force-pushed
the
topic/maintainer/file-groups
branch
from
9ea2f2f to
bb5c574
Compare
|
@decsny ok added inheritance, let see how this works out. |
nashif
force-pushed
the
topic/maintainer/file-groups
branch
from
bb5c574 to
2620ffe
Compare
|
It would be nice if the get_maintainer script commands had some more intelligence about file groups, like if the |
This new section allows defining a group of files in an area and makes it possible to assign collaborators to the file group being defined. The purpose of this new section is to allow fine tuning who is added as reviewer when files change in a group. It is especially useful in large areas with hundreds of files, for example platform areas. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Deal with new section in the maintainer file defining file groups. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Document file groups and how they should be used. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Merge two workflows into one for code sharing an efficiency. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Scripts only used by CI, so move it into that directory. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Also request reviewes from maintainers of changes areas in the maintainer file. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
File groups inherit file patterns from their parent area. A file will only match a file group if it first matches the parent area's patterns, and then also matches the file group's own patterns. This allows file groups to further filter and subdivide files that are already covered by the area. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
nashif
force-pushed
the
topic/maintainer/file-groups
branch
from
2620ffe to
fc32da0
Compare
|
| env: | ||
| GITHUB_TOKEN: ${{ secrets.ZB_PR_ASSIGNER_GITHUB_TOKEN }} | ||
| run: | | ||
| python ./scripts/ci/check_maintainer_changes.py \ |
There was a problem hiding this comment.
| python ./scripts/ci/check_maintainer_changes.py \ | |
| python ./scripts/ci/check_maintainer_changes.py \ |
There was a problem hiding this comment.
bummer, how did I miss that, release blocker :-D
There was a problem hiding this comment.
I would have added it but you asked to only do it on issues...
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for file-groups in the MAINTAINERS.yml file and improves review requests for maintainer file changes. File groups allow different collaborators to be assigned to specific subsets of files within an area, with inheritance from parent area patterns.
Key changes:
- Added file-groups support to get_maintainer.py with validation and path matching logic
- Enhanced set_assignees.py to request reviews from maintainers when their areas change
- Merged the maintainer_check workflow into the assigner workflow
- Moved set_assignees.py from scripts/ to scripts/ci/
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/get_maintainer.py | Added FileGroup class, file-groups parsing, validation logic, and path-to-collaborators methods |
| scripts/ci/set_assignees.py | Added maintainer file comparison logic, file-group-aware collaborator selection, and updated paths |
| MAINTAINERS.yml | Added file-groups documentation and updated set_assignees.py path references |
| scripts/ci/twister_ignore.txt | Updated path reference for set_assignees.py |
| .ruff-excludes.toml | Added new path for set_assignees.py |
| .github/workflows/assigner.yml | Merged maintainer check functionality and updated script path |
| .github/workflows/maintainer_check.yml | Removed entire workflow file (merged into assigner) |
Comments suppressed due to low confidence (7)
scripts/ci/set_assignees.py:90
- The parse_args() function is missing a return statement. It sets a global
argsvariable but should also return the parsed arguments for consistency with typical argparse patterns.
scripts/ci/set_assignees.py:96 - The load_areas() function filters areas based only on 'files' or 'files-regex', but according to the changes in get_maintainer.py (line 714-715), areas can now also be defined with only 'file-groups'. This will cause file-group-only areas to be excluded from maintainer file change detection.
scripts/ci/set_assignees.py:270 - Corrected spelling of 'reviewrs' to 'reviewers'.
scripts/ci/set_assignees.py:260 - The log message 'cannot process MAINTAINERS.yml changes, skipping...' is misleading. The code actually does process the changes (lines 262-270). This message should be removed or reworded to accurately reflect what's happening.
scripts/ci/set_assignees.py:305 - The collaborator collection logic is duplicated. Lines 273-274 already collect collaborators for regular files, and lines 304-305 duplicate this for sorted_areas (which is derived from the same areas list). This inner loop (304-305) appears redundant and should be removed to avoid adding the same collaborators multiple times.
scripts/ci/set_assignees.py:251 - Attempting to call get_collaborators_for_path() on a string. The variable _area is a string (area name from process_manifest), not an Area object, so this will fail. This method call should be removed or moved to after area_match is found.
scripts/ci/set_assignees.py:316 - collab_per_path is a set, but it's being concatenated with a list using +=. This will iterate through the set and add each item individually, which works but is inconsistent. Either convert collab_per_path to a list when extending, or use set operations throughout.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # match a file group if it first matches the parent area's patterns, and then | ||
| # also matches the file group's own patterns. This allows file groups to | ||
| # further filter and subdivide files that are already covered by the area. |
There was a problem hiding this comment.
Inconsistent indentation: tabs are used instead of spaces in the documentation comment. Lines 69-71 should use consistent spacing to match the rest of the file.
| # match a file group if it first matches the parent area's patterns, and then | |
| # also matches the file group's own patterns. This allows file groups to | |
| # further filter and subdivide files that are already covered by the area. | |
| # match a file group if it first matches the parent area's patterns, and then | |
| # also matches the file group's own patterns. This allows file groups to | |
| # further filter and subdivide files that are already covered by the area. |
|
@decsny so how would this look for nxp? Personally I think it'd be simpler and more flexible to just add more areas and maybe add a priority index to help the set assignee script picking one instead of another, but I guess if it works for you folks it's fine. |
See #99104 for a start attempt using it for NXP areas, which will definitely be subdivided even more in the future. And what I was saying originally is I don't understand what's the problem with having more top level areas, I personally don't care. But that apparently rubbed people the wrong way for multiple reasons (that imo were more cosmetic/pedantic reasons than actually meaningful), then it got all mixed up with the discussion about what a maintainer is, now we have this solution which everybody agreed on in process WG. My only requirement is to be able to add more NXP engineers to be requested to review more specific files that they actually work on / are responsible for, and to be able to approve/block changes to those files, without overloading the reviewer list with too many NXP people added on everything, and without upstream community throwing a fit about NXP having too many areas or "polluting the MAINTAINER file" or whatever other supposed issues. So I don't care how we get to that point, but this solution I think is fine for the goals. Thanks @nashif for helping with this. |
7 participants
