Updating changes to Approver and Reviewer

[jbottum]

@kimwnasptd I think we need to review the requirements for approver and approver, especially the number of PRs that each title must work on. Please see my suggestions in this PR.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jbottum
Once this PR has been reviewed and has the lgtm label, please assign theadactyl for approval by writing /assign @theadactyl in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[terrytangyuan]

I would clarify what substantial means:

Perhaps add something like "with the definition of substantial subject to the lead's discretion (e.g. refactors, enhancements rather than grammar correction or one-line pulls)" similar to what we are doing at Argo project.

[johnugeorge]

+1. As in the Argo doc, we can also combine these 2 points to one Reviewer for or author of at least 5 substantial PRs to the codebase,

[james-jwu]

+1 to Johnu's wording.

[jbottum]

ok, I changed to "Reviewer for or author of at least 5 substantial PRs to the codebase" in 2nd commit

[terrytangyuan]

3 substantial PRs?

[jbottum]

ok, I changed in "Reviewer for or author of at least 5 substantial PRs to the codebase" in 2nd commit

[terrytangyuan]

This only mentioned reviewing PRs. I think we want to promote people who actually authored substantial PRs.

[johnugeorge]

+1. Similar comment as before. Combining to one, Reviewer for or author of at least 10 substantial PRs to the codebase

[james-jwu]

+1 to Johnu's wording.

[jbottum]

ok, I updated to "Reviewer for or author of at least 5 substantial PRs to the codebase" in 2nd commit

[kimwnasptd]

Thanks for starting this effort @jbottum! I'll take a more detailed look within the week and add some more concrete comments.

The first thing I believe we should improve is the fact that we have a lot of information exposed in different places of kubeflow/community

• We define the WGs and their leads (tech leads, chairs, subproject owners) in https://github.com/kubeflow/community/blob/master/wgs.yaml
• We expose (duplicate?) information regarding the WG list in https://github.com/kubeflow/community/blob/master/wg-list.md
• We have the guidelines on how someone can climb the "WG ladder" in https://github.com/kubeflow/community/blob/master/wgs/community-membership.md
• We explain what the responsibilities of leadership positions are in https://github.com/kubeflow/community/blob/master/wgs/wg-governance.md

We should try and have "tracking doc" that ties all these together. This will make it easier for new contributors to understand the governance structure and expectations. That could even be a page in the kubeflow.org site, to have even more exposure.

Then regarding the file in this PR (wgs/community-membership.md) what I believe is missing is:

• Expose how the subproject owners map to the other files (IIUC it's the workinggroups[i].subprojects[x].owners in wgs.yaml
• Also expose how someone can become a WG Tech Lead from a subproject owner
• Expose how a WG Tech Lead can become a chair

[kimwnasptd]

I see that folks also mention using terminology and processes from other projects. While I really like the idea of pick and choosing concepts, like identifying inactive contributors, I believe that for now we just need to make sure our current roles/governance-model is exposed in a central visible place and explained in "simple terms".

While I do really like how Argo has a more simplified model (they have just the term of a subproject lead, which represents OWNERS in repos) I think it would introduce too much friction right now to change our roles.

To close this, I'll just end with exposing my current understanding about approvers/reviewers and all the way to WG Chairs:

• reviewers in folders of a GH repo (subproject) are responsible for the quality of code. We should be relatively lax on allowing members to become reviewers
• approvers in folders of a GH repo (subproject) are responsible for technical decision making for the code they "own". We need to identify in this PR the threshold and criteria on when a reviewer should become and approver (i.e. commitment measured in PRs/comments per months, number of PRs, showing technical leadership etc)
• subproject owners, or simply put approvers in the root OWNERS files of a Kubeflow project. At least this is my understanding from the subprojects section in the wgs.yaml
  • We need a clear path on when should an approver in sub-folders of a repo be added in the "root" OWNERS file
  • Make sure we document that the subproject owner role is mostly technical
• WG Tech Lead is someone that has technical expertise in the subprojects of a WG. They are responsible to distinguish technical problems in the code architecture and work with the WG Chairs on creating a ROADMAP for the WG and breaking down/prioritizing tasks https://github.com/kubeflow/community/blob/master/wgs/wg-governance.md#tech-lead
• WG Chair is responsible for setting the high level direction of the WG, by working alongside the WG Tech Leads to ensure the agreed goals are viable and decide where to prioritize https://github.com/kubeflow/community/blob/master/wgs/wg-governance.md#chair

Reminder that right now a WG Lead is considered someone that is either a Tech Lead, a Chair, or a Subproject Owner https://github.com/kubeflow/community/blob/master/wgs/wg-governance.md#notes-on-roles

Within this section "Lead" refers to someone who is a member of the union of a Chair, Tech Lead or Subproject Owner role

[zijianjoy]

To @jbottum : I would like to learn more about the reason behind this PR change: is there an individual or a group of individuals who we are trying to promote as reviewer/approver, but they are blocked due to current PR count requirements? Can WG lead make a collective decision to sponsor such individual?

A sidenote: I think @kimwnasptd 's comment in this PR is helpful in terms of exposing current role structure in a centralized place in simple term at the current timeframe, we still have time to improve the detail of role structure after donating to CNCF.

[jbottum]

@zijianjoy this proposed PR change was intended to 1) make sure the WG leads have reviewed the requirements for approver and reviewer 2) find the smallest change possible that is still operational in practice. I am supportive of the ideas in this thread and would like to get to a concrete consensus on something we can understand, explain and follow.

[theadactyl]

Agree with Kimonas that the focus right now should be on better defining/documenting roles, rather than making any changes to our roles. Governance changes should be managed by KSC once established.

…

On Thu, Dec 15, 2022 at 8:25 PM Josh Bottum ***@***.***> wrote: @zijianjoy <https://github.com/zijianjoy> this proposed PR change was intended to 1) make sure the WG leads have reviewed the requirements for approver and reviewer 2) find the smallest change possible that is still operational in practice. I am supportive of the ideas in this thread and would like to get to a concrete consensus on something we can understand, explain and follow. — Reply to this email directly, view it on GitHub <#591 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABREJVLD4XUOGQIMQV56R6DWNPVKHANCNFSM6AAAAAAS3JTPGA> . You are receiving this because your review was requested.Message ID: ***@***.***>

[jbottum]

@kimwnasptd @johnugeorge @andreyvelich In your opinion, are we using the sub project position? Should we delete it to make things more simple ?

[juliusvonkohout]

since we want to have more hands to help with the community stuff, how can we move forward here?