OWNERS: update to use new labels #2444
Conversation
Ensure the are OWNERS files that will auto-label PRs with
area/apps/{foo} for every apps/{foo} subdirectory
For apps that didn't have OWNERS, I took arbitrary guesses based on
group membership in the corresponding RBAC group, or knowledge of who
had been merging PRs related to the apps recently.
I also dropped the "no_parent_owners" option for k8s.io since the app is
now auto-deployed on PR merge.
Ensure OWNERS files exist to auto-label PRs in high-traffic or high-value directories. For directories that didn't have OWNERS, I made educated guesses: - things related to artifact promotion got a similar set of approvers/reviewers as uesd by the image manifests for container-image-promoter - namespaces is sorta apps-centric, same people
I _think_ the code-of-conduct and steering committee OWNERS files were invalid, I'm surprised we didn't get a presubmit failure or invalid-owners label for them before.
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: spiffxp 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 |
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
area/access
|
/hold |
k8s-ci-robot
added
sig/security
| - munnerz | ||
| - munnerz |
There was a problem hiding this comment.
So, one change that happened a lot was trying to make formatting consistent:
- deindent list items
- newline between approver/reviewer/emeritus_approver lists (and put them in that order)
| # deploying this is still a manual process, so restricting | ||
| # to approvers who are comfortable doing so | ||
| options: | ||
| no_parent_owners: true | ||
|
|
There was a problem hiding this comment.
Now that we have jobs auto-deploy k8s.io changes on PR merge, this is no longer needed 🎉
| - wg-k8s-infra-leads | ||
| - ameukam | ||
| - spiffxp |
There was a problem hiding this comment.
AFAIK @ameukam and I are the only two who have domain expertise on wg-k8s-infra's use of this, I'd rather signal that to the other leads
| emeritus_approvers: | ||
| - bartsmykla |
There was a problem hiding this comment.
AFAIK we haven't seen much approval from this contributor over the last while so it's time to emeritus
| - area/prow/bump | ||
| - sig/testing | ||
| - area/apps/prow |
There was a problem hiding this comment.
Formatting change: for apps, reorder so the "owning group" label is first, and the "app-specific" label is second
| approvers: | ||
| - ameukam | ||
| - spiffxp | ||
| - thockin |
There was a problem hiding this comment.
AFAIK these are the only people who have reviewed the automated PRs here
| reviewers: | ||
| - nikhita |
There was a problem hiding this comment.
@nikhita has helped get us on the path to delegated approval
| committee-code-of-conduct | ||
| - committee-code-of-conduct |
There was a problem hiding this comment.
I'm not sure this OWNERS file ever worked, so I converted to list items to fix
| # be explicit about looking for $(pwd)/policy | ||
| --policy "${REPO_ROOT}/policy" |
There was a problem hiding this comment.
To match the new area/policy label, I relocated policies/* to policy/ which should just work by default, but I felt like staying explicit was better
| approvers: | ||
| - cip-approvers | ||
| - wg-k8s-infra-leads | ||
| reviewers: | ||
| - cip-reviewers | ||
| - release-engineering-approvers | ||
| - release-engineering-reviewers |
There was a problem hiding this comment.
I used k8s.gcr.io/images/k8s-staging-artifact-promoter/OWNERS as the basis for other CIP-related stuff
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
Related:
This PR is primarily about using the new labels defined in kubernetes/test-infra#23084 to help us better triage incoming PRs
But of course, all of the commits have scarily vague "update OWNERS" messages. I added details in the commits to explain what else was going on, but I'll drop a review on this pointing out specifics