-
Notifications
You must be signed in to change notification settings - Fork 317
Add distinct owners files to reflect the shared ownership of the repo #2997
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
base: master
Are you sure you want to change the base?
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: swetharepakula The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
690927a to
79cf228
Compare
* L4, NEG, PSC are owned by one team * L7 is owned by one team
79cf228 to
41a3f48
Compare
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| # approvers for L7 features | ||
| approvers: | ||
| - l7-owners | ||
| - general-maintainers | ||
| reviewers: | ||
| - l7-owners | ||
| - general-maintainers |
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.
L4 has most of the code here. I don't think there could be a single owner for this pkg, unless one side moves out from it
| l4-neg-psc-owners: | ||
| - mmamczur | ||
| - tortillazhawaii | ||
| - felipeyepez |
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.
Enrico has received Kubernetes Membership. #3003
| - felipeyepez | |
| - felipeyepez | |
| - 08volt |
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.
It probably causes the merge conflict as well
| - aojea | ||
| - thockin | ||
| - bowei | ||
| - mrhohn |
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.
We shouldn't strip mrhohn, panslava, cezarygerard, code-elinka, gauravkghildiyal, and sawsa307 of their privileges on the occasion of splitting ownership for distinct controllers. If some of these names are outdated, it can be handled in separate PRs with the relevant folks as 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.
A good solution might be to use emeritus_approvers https://go.k8s.io/owners
| - l7-owners | ||
| - l4-neg-psc-owners |
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.
I think that adding l7-owners and l4-neg-psc-owners to the root OWNERS file will cause other OWNERS files to inherit from their parent directories. My guess is that this change would make all groups owners of every subdirectory in the repository even if other directories specify a different config.
From what I've seen ownership is additive by default so the OWNERS permissions from the parent directory are combined with the permissions in the subdirectory's OWNERS file. An owner in a parent directory is still an owner in a child directory. Might be worth checking since I can be wrong
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 solution might be to use no_parent_owners on subdirectory OWNERS files https://go.k8s.io/owners
| - general-maintainers | ||
| reviewers: | ||
| - l4-neg-psc-owners | ||
| - general-maintainers |
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.
If my comment about ownership being additive is true we might not need to specify general-maintainers in every file
/assign @kl52752
/assign @mmamczur
/hold Do not merge until both assignees LGTM