-
Notifications
You must be signed in to change notification settings - Fork 43
Update code-ownership files to best utilize PROW + auto assign #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Maroon Ayoub <[email protected]>
Signed-off-by: Maroon Ayoub <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reorganizes code ownership and review assignment by introducing proper PROW-compatible ownership files and restructuring the CODEOWNERS file for automatic PR assignment.
- Introduces
OWNERS_ALIASES
to define maintainer and reviewer groups - Updates
OWNERS
to use alias references instead of individual usernames - Restructures
CODEOWNERS
to focus on automatic PR assignment with granular package-specific assignments
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
OWNERS_ALIASES | Defines maintainer and reviewer groups for PROW integration |
OWNERS | Updates approvers and reviewers to reference alias groups |
CODEOWNERS | Restructures for automatic PR assignment with clearer purpose |
.licenserc.yaml | Adds OWNERS_ALIASES to license check exclusions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- llm-d-kv-cache-manager-maintainers | ||
- llm-d-kv-cache-manager-reviewers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The maintainers group is listed in both approvers and reviewers sections. This creates redundancy since approvers typically have reviewer permissions by default. Consider removing the maintainers group from the reviewers list to avoid duplication.
- llm-d-kv-cache-manager-maintainers | |
- llm-d-kv-cache-manager-reviewers | |
- llm-d-kv-cache-manager-reviewers |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately that is not true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Summary
OWNERS
andOWNERS_ALIASES
for maintainer/reviewers maintenance + PROW permissionsCODEOWNERS
for automatic PR assignment. Note that PROW'sblunderbuss
was not used since it does not support granular assignment (e.g.,guygir
for thepreprocessing
package)Once team management is more convenient in llm-d (cc @Gregory-Pereira perhaps?), teams can be synced with
OWNERS_ALIASES
forCODEOWNERS
to also reference them.