-
Notifications
You must be signed in to change notification settings - Fork 398
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
Add owners files and notes on review/approver role to CONTRIBUTING.md #1601
Conversation
please 👍 here to indicate you're good with what is shown in this PR with regards to the code areas and responsibilities. Thank you! |
I mostly agree with the proposal. One change I think that is important:
|
See how an API approver group can be implemented: |
ack, thanks for the feedback. Follow up question: do you think the additional people listed in admission packages should be added as reviewers or removed? Since I didn't state it earlier I think this commit should get both +1 from @sttts and @ncdc as repo owners as well as a +1 by anyone listed in the file that they're comfortable with the role. |
Definitely as reviewers. We should encourage reviewers everywhere, especially those who have written the majority of code in a package should be reviewer there. |
dbc7dde
to
e00dbce
Compare
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.
@pweil-: 1 invalid OWNERS file
In response to this:
Fixes #1557
Figured this might be easier with something to actually apply red pen to. Note, this is not a judgement of any contributor's capabilities. I simply looked at history and picked what I thought looked like names that were frequently contributing to areas. Comment and change suggestions encouraged.
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/test-infra repository.
Co-authored-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>
lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts 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 |
LGTM. What's left before merging? |
Folks were pinged on Slack today to give final thumbs up on where there names are. We can merge EOD tomorrow to give that time. Otherwise, no big deal, we can change it later. |
/lgtm @pweil- feel free to remove the hold at your EoD. |
unholding. For any further feedback please submit it in the form of a PR. 👍 |
Fixes #1557
Figured this might be easier with something to actually apply red pen to. Note, this is not a judgement of any contributor's capabilities. I simply looked at history and picked what I thought looked like names that were frequently contributing to areas. Comment and change suggestions encouraged.