Org: Define and implement entry/exit criteria for kubernetes-maintainers GitHub team

Created on 18 Nov 2020  路  23Comments  路  Source: kubernetes/org

The legacy kubernetes-maintainers GitHub team has write access to the k/k repo. This issue is to discuss pruning the team.

There has been previous discussion about _removing_ the team in https://github.com/kubernetes/kubernetes/issues/57667. Given that we don't have good automated solutions around editing issue/PR bodies, completely removing write access and deleting the team is not feasible today.

Having said that, the kubernetes-maintainers team hasn't had any significant updates over the last ~3 years (!!!). There are members in that team who are no longer active in the Kubernetes project and should not have write access to k/k anymore.

My suggestion - we could limit this to milestone-maintainers + all approvers in k/k.

If there is general consensus on this GitHub issue, I'll start a thread on the k-dev and leads mailing lists.

aregithub-permissions help wanted kincleanup kindocumentation prioritimportant-soon siarchitecture sicontributor-experience sirelease

Most helpful comment

To clarify - we won't bulk-add milestone-maintainers + k/k approvers if they aren't already in the kubernetes-maintainers GitHub team.

If someone in milestone-maintainers + k/k approvers wants to be on the team, they can create a PR to add themselves separately.

Want to keep the team as small as possible.

All 23 comments

cc @kubernetes/owners
GitHub admin team

cc @dims @derekwaynecarr @johnbelamaric
sig-arch leads

cc @liggitt @sttts @deads2k @tpepper @justaugustus
who've had opinions about this team before (in https://github.com/kubernetes/kubernetes/issues/57667 + https://github.com/kubernetes/org/pull/2330)

+1. It's been needed for some time.

+1 to prune. isn't milestone-maintainers a larger group?

+1

My suggestion - we could limit this to milestone-maintainers + all approvers in k/k.

Is this suggesting we remove current members of the kubernetes-maintainers list if either of the following are true:

  • they are not in milestone-maintainers
  • they are not an approver in k/k

If so, that seems very reasonable to me and moves in the desired direction of shrinking the list.

Is this suggesting we remove current members of the kubernetes-maintainers list if either of the following are true:

  • they are not in milestone-maintainers
  • they are not an approver in k/k

Yes :+1:

To clarify - we won't bulk-add milestone-maintainers + k/k approvers if they aren't already in the kubernetes-maintainers GitHub team.

If someone in milestone-maintainers + k/k approvers wants to be on the team, they can create a PR to add themselves separately.

Want to keep the team as small as possible.

@nikhita @liggitt sounds like a good plan. Let's do this!

One note

If someone in milestone-maintainers + k/k approvers wants to be on the team, they can create a PR to add themselves separately.

What would be the approval process here?
My one concern is that Release Team members (specifically Release Team shadows) are on the milestone-maintainers group and I'm not in favor of them getting write access to k/k without some process in place.

Thinking more about this, how about:

  • If an existing member of kubernetes-maintainers is not a k/k approver, we remove them from the team
  • We don't add any new members to the team
* If an existing member of `kubernetes-maintainers` is not a k/k approver, we remove them from the team

I like this.

* We don't add any new members to the team

Hmmmmm, so I think there should be a route to write access.

@dims is not a root approver for k/k, but I do feel he's deserving of the access that was granted in https://github.com/kubernetes/org/pull/2330.

How can we cover those cases in future?

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

/remove-lifecycle stale

NOTE: due to branch protection write isn't really write to the repo unless you are in the admin superset which is _very_ small / restricted.

We should prune inactive members.

I disagree that we should refuse to add anyone new. Being able to manually correct PR state (PR body, labels) is quite useful, and we should be able to trust people with this (approvers or maybe SIG chairs / leads).

EDIT: write permission gates pretty much any mutable state of any sort (labels, titles, milestones, etc.), well beyond the repo contents. While branch protection turns around and restricts the part of write most people probably think about (pushing/merging to branches).

What requirements do triagers have that could not be covered with the Triage role?

Triage: Recommended for contributors who need to proactively manage issues and pull requests without write access

ref: https://docs.github.com/en/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization

I'm fine trying the triage role but would prefer we ensure peribolos has support for that. Unclear to me how actively it's used elsewhere

https://kubernetes.slack.com/archives/C1L57L91V/p1614017710014100 based on k/enhancements experience with the triage role, I don't think it meets our needs here (inability to edit issue/pr descriptions)

We need to decide on policy to prune, and policy to add. Then implement. Summarizing what I've read above from @nikhita @liggitt @BenTheElder @justaugustus

Possible reasons to prune:

  • A: is not an approver somewhere in k/k
  • B: is not a member of milestone-maintainers
  • C: has not used superpowers in previous N months (superpowers: edit issue/title description, since this is something Triage cannot do)
  • D: all of the above

Would like to understand what the reduction would be based on each of these. My preference would be C. If only I knew how to measure it at a glance...

Requirements to be added:

  • A: !(A or B from above) aka "is an approver somewhere in k/k AND is a member of milestone-maintainers"
  • B: +1 from a root approver
  • C: all of the above
  • D: none of the above, nobody new gets added

My preference would be C

I don't think Triage as a role is really helpful for us:

  • Apply/dismiss labels - it can do this, so it can bypass OWNERS by directly adding the approved label
  • Edit and delete anyone's comments on commits, pull requests, and issues - it can't do this, so it wouldn't help correct release-note foibles

/retitle Define and implement entry/exit criteria for kubernetes-maintainers GitHub team

/help
We could really use someone's help in measuring the impact of reasons to prune: https://github.com/kubernetes/org/issues/2332#issuecomment-795660174

A: parse all OWNERS files in kubernetes/kubernetes

  • I have done this in the past with find and yq in a pinch, or python with glob and ruamel.yaml when I needed to parse OWNERS_ALIASES too
  • Another maybe easier approach is write golang that uses k8s.io/test-infra/prow/repoowners

B: diff the two teams' membership

  • can use files in kubernetes/org, or github's api

C: this is the one I'm not sure about

  • does a github search query for involves: spiffxp pick up issues I have edited (but not commented on, reviewed, or been mentioned)? -> can implement script based on "what this member has done on issues/PRs updated in last N months"
  • is there a GitHub API way of identifying edits to an issue/PR?

D: intersection of the above

  • if all done in the same script/golang, whatever boolean logic suits you best
  • silly bash way of doing intersection: dump "who gets pruned" into a file for each of the above, then cat pruned-by-[ABC].txt | sort | uniq -c | grep '^[ ]*3

@spiffxp:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help
We could really use someone's help in measuring the impact of reasons to prune: https://github.com/kubernetes/org/issues/2332#issuecomment-795660174

A: parse all OWNERS files in kubernetes/kubernetes

  • I have done this in the past with find and yq in a pinch, or python with glob and ruamel.yaml when I needed to parse OWNERS_ALIASES too
  • Another maybe easier approach is write golang that uses k8s.io/test-infra/prow/repoowners

B: diff the two teams' membership

  • can use files in kubernetes/org, or github's api

C: this is the one I'm not sure about

  • does a github search query for involves: spiffxp pick up issues I have edited (but not commented on, reviewed, or been mentioned)? -> can implement script based on "what this member has done on issues/PRs updated in last N months"
  • is there a GitHub API way of identifying edits to an issue/PR?

D: intersection of the above

  • if all done in the same script/golang, whatever boolean logic suits you best
  • silly bash way of doing intersection: dump "who gets pruned" into a file for each of the above, then cat pruned-by-[ABC].txt | sort | uniq -c | grep '^[ ]*3

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.

/help
We could really use someone's help in measuring the impact of reasons to prune: #2332 (comment)

@kubernetes/release-engineering -- Anyone interested in helping out here?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jmyung picture jmyung  路  3Comments

cblecker picture cblecker  路  3Comments

RA489 picture RA489  路  3Comments

ibrasho picture ibrasho  路  3Comments

hpandeycodeit picture hpandeycodeit  路  3Comments