From 11fc419cee39ebe1b21263f6c24c050f3b418ddf Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Fri, 22 Sep 2023 13:14:48 -0500 Subject: [PATCH] be more specific about reqs in contributing guide (#5330) * be more specific about reqs in contributing guide * fix wording --- CONTRIBUTING.md | 128 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 114 insertions(+), 14 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e9174c5b1..4f1bde1f4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -59,6 +59,113 @@ Kustomize follows the [Kubernetes Community Membership] contributor ladder. Role The kyaml module within the Kustomize repo has additional owners following the same ladder. +For the kustomize project, we have defined some specific guidelines on each step of the ladder: + +To reach reviewer status, you must: +- Have been actively involved in kustomize for 3+ months +- Review at least 8 PRs that have been driven through to completion (see the reviewer guide below) +- Author at least 5 PRs that have been approved and merged +- Be a member of the kubernetes-sigs org. This should not be a blocker though, as once you meet the requirements for reviewer here, +the existing kustomize maintainers will be happy to sponsor your request to join the kubernetes-sigs org. +- Once you have met the above requirements, you may submit a PR adding yourself to the kustomize reviewers list, with links to your +contributions in the description. + +To reach approver status, you must: +- Meet all the requirements of a reviewer +- Have been actively involved in kustomize for 6+ months +- Review at least 15 PRs that have been driven through to completion (see the reviewer guide below) +- Authored PRs meeting *either* of the following requirements: + - 15 PRs that have been approved and merged + - *OR* 10 PRs that have been approved and merged where some were more difficult, required greater thought/design, +or built up to larger features/long-term goals. +- File 3 issues. This can be any number of things, including but not limited to: + - Bugs with kustomize usage that you've found + - CI or release improvements + - Creating subtasks of a larger feature or project that you are in charge of. + - Long term improvements for the health of the project +- Triage at least 10 untriaged issues, including at least 1 feature request. The kustomize bug scrub is a great place to get practice with doing this, but you can +also follow the triage guide below to get started on your own. +- Demonstrate deeper understanding of kustomize goals. This can take many forms and is a bit subjective, but here are a few examples: + - saying no to an eschewed feature, instead recommending an alternative solution that is more aligned with the declarative configuration model + - active participation in discussion on a feature request issue + - filing an issue describing a long term problem and solution aligned with kustomize goals, for example: https://github.com/kubernetes-sigs/kustomize/issues/5140 + - writing up KEPs for features that will improve the kustomize workflow while being aligned with kustomize goals, for example: https://github.com/kubernetes-sigs/kustomize/pull/4558 +- Regularly interact with the existing kustomize maintainers, with clear communication about what you are working on or planning to work on. The kustomize +maintainers should know who you are and be familiar with your contributions. +- If you meet *most* of the above requirements while going above and beyond in a few areas, we will still consider your request to become an approver even +if you are missing one or two of the requirements. Please contact the maintainers directly to ask about getting approver status if you fall into this category. +- Otherwise, once you meet all the above requirements, you may: + - request to be added to the kustomize maintainer meeting that occurs each week with the kustomize PMs. + - submit a PR adding yourself to the kustomize approvers list, with links to your contributions in the description. + +To reach owner status, you must: +- Meet all the requirements of an approver +- Have been actively involved with kustomize for 1+ year +- Assisted the current owner in driving the roadmap. This can be explicit or implicit help, such as: + - Editing the roadmap directly + - Reviewing the roadmap + - Providing suggestions for issues or prioritization in meetings that indirectly influence the roadmap +- Regularly triage issues and attend the kustomize bug scrub +- Regularly review PRs (1-2 a week) +- Periodically lead the kustomize bug scrub +- Periodically release kustomize (ensuring that there are no release blockers and that release notes are clean) +- Be the primary owner or point of contact for a particular project or area of code +- Ideally, there should be 2-3 owners at a time. Reach out to the current owners if you are interested in ownership. These +requirements are not strict and evaluation is somewhat subjective. + +## Reviewer guide +Please watch this talk on how to review code from Tim Hockin: https://www.youtube.com/watch?v=OZVv7-o8i40 + +For reviewing PRs in kustomize, we have some specific guidelines: +- If the PR is introducing a new feature: + - *It must be implementing an issue that has already been triage/accepted or +a KEP that has been approved.* If it is not, then request the PR author to first file an issue. + - The PR must include thorough tests for the new feature, including unit and integration tests + - The code must be clean and readable, with thought given to how we will maintain the code in the future + - If the feature requires being broken up into multiple PRs to ease review, the feature should not be exposed to users + until the feature is completed in the last PR. For example, while we were building `kustomize localize`, we + built the feature almost entirely under the `api` module as a library with all the needed tests. There was no way + for users to invoke the localize code until the last PR that actually exposed the `kustomize localize` command in the + kustomize binary. This allowed us to continue development of `kustomize localize` without blocking kustomize releases. + If this type of development is not possible, then new features requiring multiple PRs should be + developed in their own feature branch. +- If the PR is introducing a bug fix: + - If the PR is not fixing an issue that has already been triage/accepted, follow the triage guide below on bug + fixes to decide if this is a PR we want to accept. + - The PR should have two distinct commits: + - The first commit should add a test demonstrating incorrect behavior + - The second commit should include the bug fix + - Some sample PRs: + - https://github.com/kubernetes-sigs/kustomize/pull/5263/commits + - https://github.com/kubernetes-sigs/kustomize/pull/3931/commits + - The regression test is absolutely required, and we cannot accept bug fixes without tests. +- If the PR is introducing a performance improvement: + - The PR description should give an indication of how much the performance is being improved and how we + can measure it - benchmark tests are fantastic. +- Other PRs (documentation, CI improvements, etc.) should be reviewed based on your best judgment. + +## Triage guide +The possible triage labels are listed here: https://github.com/kubernetes-sigs/kustomize/labels?q=triage. + +Triaging a feature request means: +- Understand what the user is asking for, and their use case. +- Verify that it is not an [eschewed feature](https://kubectl.docs.kubernetes.io/faq/kustomize/eschewedfeatures/#build-time-side-effects-from-cli-args-or-env-variables) +- Verify that it is not a duplicate issue. +- Look into workarounds. Is there another way that the user can achieve their use case with existing features? +- If you are new to this role, prior to leaving a comment on the issue, please bring it to weekly standup +for group discussion to make sure that we are all on the same page. +- Once you feel ready, you can label it with a triage label. Here's an [example](https://github.com/kubernetes-sigs/kustomize/issues/5049). You can also +look at other feature request issues to see how they were triaged and resolved. There are a few different triage labels that you can use, you can see the +full list [here](https://github.com/kubernetes-sigs/kustomize/labels?q=triage). + +Triaging a bug means: +- First, verify that you can reproduce the issue. If you cannot reproduce the issue or need more information to give +it a go, triage it accordingly. +- Try to understand if this is really a bug or if this is intended behavior from kustomize. If it seems like intended +behavior, do your best to explain to the user why this is the case. +- If it seems to be a genuine bug, you can /triage accept the issue. In addition, investigate if there are workarounds or +alternative solutions for the user that they can try until the issue gets resolved. + Administrative notes: - The [OWNERS file spec] is a useful resources in making changes. @@ -66,23 +173,16 @@ Administrative notes: ## Project/Product Managers -Kustomize will have opportunities to join in a project/product manager role. You can -typically start working on this role as part of a kustomize training cohort, so please -keep an eye out for that or reach out to the leads if you are interested in this type of -work. Expectations for this role are: +Kustomize will have opportunities to join in a project/product manager role. You can reach out to +the existing kustomize maintainers if you are interested in this type of role. Project management work +can greatly help supplement your contributions as you climb from reviewer to approver +to owner. + +Expectations for this role are: - Triage 1 feature request each week, and bring it to weekly stand-up for discussion. Feature requests are issues labeled kind/feature, and you can find them [here](https://github.com/kubernetes-sigs/kustomize/issues?q=is%3Aissue+is%3Aopen+kind+feature+label%3Akind%2Ffeature). -Triaging a feature request means: - - Understand what the user is asking for, and their use case. - - Verify that it is not an [eschewed feature](https://kubectl.docs.kubernetes.io/faq/kustomize/eschewedfeatures/#build-time-side-effects-from-cli-args-or-env-variables) - - Verify that it is not a duplicate issue. - - Look into workarounds. Is there another way that the user can achieve their use case with existing features? - - If you are new to this role, prior to leaving a comment on the issue, please bring it to weekly standup - for group discussion to make sure that we are all on the same page. - - Once you feel ready, you can label it with a triage label. Here's an [example](https://github.com/kubernetes-sigs/kustomize/issues/5049). You can also -look at other feature request issues to see how they were triaged and resolved. There are a few different triage labels that you can use, you can see the -full list [here](https://github.com/kubernetes-sigs/kustomize/labels?q=triage). +Please view the above triage guide for details on how to approach feature request triage. - Monitor the kustomize Slack channel and try to help users if you can. It is a pretty active channel, so responding to 4-5 users per week is sufficient even if some questions go unanswered. If there is an interesting topic or a recurring problem that many