|
| 1 | +Contributing Roles |
| 2 | +================== |
| 3 | + |
| 4 | +## Direct Code-Related Roles |
| 5 | + |
| 6 | +While anyone (who's signed the [CLA and follows the code of |
| 7 | +conduct](../CONTRIBUTING.md)) is welcome to contribute to the KubeBuilder |
| 8 | +project, we've got two "formal" roles that carry additional privileges and |
| 9 | +responsibilities: *reviewer* and *approver*. |
| 10 | + |
| 11 | +In a nutshell, reviewers and approvers are officially recognized to make |
| 12 | +day-to-day and overarching technical decisions within parts of the |
| 13 | +project, or the project as a whole. We follow a similar set of |
| 14 | +definitions to the [main Kubernetes project itself][kube-ladder], with |
| 15 | +slightly looser requirements. |
| 16 | + |
| 17 | +As much as possible, we want people to help take on responsibility in the |
| 18 | +project -- these guidelines are attempts to make it *easier* for this to |
| 19 | +happen, *not harder*. If you've got any questions, just reach out on |
| 20 | +Slack to one of the [subproject leads][kb-leads] (called |
| 21 | +kubebuilder-admins in the `OWNERS_ALIASES` file). |
| 22 | + |
| 23 | +## Prerequisite: Member |
| 24 | + |
| 25 | +Anyone who wants to become a reviewer or approver must first be a [member |
| 26 | +of the Kubernetes project][kube-member]. The aforementioned doc has more |
| 27 | +details, but the gist is that you must have made a couple contributions to |
| 28 | +some part of the Kubernetes project -- *this includes KubeBuilder and |
| 29 | +related repos*. Then, you need two existing members to sponsor you. |
| 30 | + |
| 31 | +**If you've contributed a few times to KubeBuilder, we'll be happy to |
| 32 | +sponsor you, just ping us on Slack :-)** |
| 33 | + |
| 34 | +## Reviewers |
| 35 | + |
| 36 | +Reviewers are recongized as able to provide code reviews for parts of the |
| 37 | +codebase, and are entered into the `reviewers` section of one or more |
| 38 | +`OWNERS` files. You'll get auto-assigned reviews for your area of the |
| 39 | +codebase, and are generally expected to review for both correctness, |
| 40 | +testing, general code organization, etc. Reviewers may review for design |
| 41 | +as well, but approvers have the final say on that. |
| 42 | + |
| 43 | +Things to look for: |
| 44 | + |
| 45 | +- does this code work, and is it written performantly and idomatically? |
| 46 | +- is it tested? |
| 47 | +- is it organized nicely? Is it maintainable? |
| 48 | +- is it documented? |
| 49 | +- does it need to be threadsafe? Is it? |
| 50 | +- Take a glance at the stuff for approvers, if you can. |
| 51 | + |
| 52 | +Reviewers' `/lgtm` marks are generally trusted by approvers to mean that |
| 53 | +the code is ready for one last look-over before merging. |
| 54 | + |
| 55 | +### Becoming a Reviewer |
| 56 | + |
| 57 | +The criteria for becoming a reviewer are: |
| 58 | + |
| 59 | +- Give 5-10 reviews on PRs |
| 60 | +- Contribute or review 3-5 PRs substantially (i.e. take on the role of the |
| 61 | + defacto "main" reviewer for the PR, contribute a bugfix or feature, etc) |
| 62 | + |
| 63 | +Usually, this will need to occur within a single repository, but if you've |
| 64 | +worked on a cross-cutting feature, it's ok to count PRs across |
| 65 | +repositories. |
| 66 | + |
| 67 | +Once you meet those criteria, submit yourself as a reviewer in the |
| 68 | +`OWNERS` file or files that you feel represent your areas of knowlege via |
| 69 | +a PR to the relevant repository. |
| 70 | + |
| 71 | +## Approvers |
| 72 | + |
| 73 | +Approvers provide the final say as to whether a piece of code is merged. |
| 74 | +Once approvals (`/approve`) are given for each piece of the affected code |
| 75 | +(and a reviewer or approver has added `/lgtm`), the code will merge. |
| 76 | + |
| 77 | +Approvers are responsible for giving the code a final once-over before |
| 78 | +merge, and doing an overall design/API review. |
| 79 | + |
| 80 | +Things to look for: |
| 81 | + |
| 82 | +- Does the API exposed to the user make sense, and is it easy to use? |
| 83 | +- Is it backwards compatible? |
| 84 | +- Will it accommodate new changes in the future? |
| 85 | +- Is it extesnible/layerable (see [DESIGN.md](../DESIGN.md))? |
| 86 | +- Does it expose a new type from `k8s.io/XYZ`, and, if so, is it worth it? |
| 87 | + Is that piece well-designed? |
| 88 | + |
| 89 | +**For large changes, approvers are responsible for getting reasonble |
| 90 | +consensus**. With the power to approve such changes comes the |
| 91 | +responsibility of ensuring that the project as a whole has time to discuss |
| 92 | +them. |
| 93 | + |
| 94 | +### Becoming an Approver |
| 95 | + |
| 96 | +All approvers need to start out as reviewers. The criteria for becoming |
| 97 | +an approver are: |
| 98 | + |
| 99 | +- Be a reviewer in the area for a couple months |
| 100 | +- Be the "main" reviewer or contributor for 5-10 substantial (bugfixes, |
| 101 | + features, etc) PRs where approvers did not need to leave substantial |
| 102 | + additional comments (i.e. where you were acting as a defacto approver). |
| 103 | + |
| 104 | +Once you've met those criteria, you can submit yourself as an approver |
| 105 | +using a PR that edits the revelant `OWNERS` files appropriately. The |
| 106 | +existing approvers will then approve the change with lazy consensus. If |
| 107 | +you feel more comfortable asking before submitting the PR, feel free to |
| 108 | +ping one of the [subproject leads][kb-leads] (called kubebuilder-admins in |
| 109 | +the `OWNERS_ALIASES` file) on Slack. |
| 110 | + |
| 111 | +## Indirectly Code-Related/Non-Code Roles |
| 112 | + |
| 113 | +We're always looking help with other areas of the project as well, such |
| 114 | +as: |
| 115 | + |
| 116 | +### Docs |
| 117 | + |
| 118 | +Docs contributors are always welcome. Docs folks can also become |
| 119 | +reviewers/approvers for the book by following the same process above. |
| 120 | + |
| 121 | +### Triage |
| 122 | + |
| 123 | +Help triaging our issues is also welcome. Folks doing triage are |
| 124 | +responsible for using the following commands to mark PRs and issues with |
| 125 | +one or more labels, and should also feel free to help answer questions: |
| 126 | + |
| 127 | +- `/kind {bug|feature|documentation}`: things that are broken/new |
| 128 | + things/things with lots of words, repsectively |
| 129 | + |
| 130 | +- `/triage support`: questions, and things that might be bugs but might |
| 131 | + just be confusion of how to use something |
| 132 | + |
| 133 | +- `/priority {backlog|important-longterm|important-soon|critical-urgent}`: |
| 134 | + how soon we need to deal with the thing (if someone wants |
| 135 | + to/eventually/pretty soon/RIGHT NOW OMG THINGS ARE ON FIRE, |
| 136 | + respectively) |
| 137 | + |
| 138 | +- `/good-first-issue`: this is pretty straightforward to implement, has |
| 139 | + a clear plan, and clear criteria for being complete |
| 140 | + |
| 141 | +- `/help`: this could feasibly still be picked up by someone new-ish, but |
| 142 | + has some wrinkles or nitty-gritty details that might not make it a good |
| 143 | + first issue |
| 144 | + |
| 145 | +See the [Prow reference](https://prow.k8s.io/command-help) for more |
| 146 | +details. |
| 147 | + |
| 148 | +[kube-ladder]: https://github.com/kubernetes/community/blob/master/community-membership.md "Kubernetes Community Membership" |
| 149 | + |
| 150 | +[kube-member]: https://github.com/kubernetes/community/blob/master/community-membership.md#member "Kubernetes Project Member" |
| 151 | + |
| 152 | +[kb-leads]: ../OWNERS_ALIASES "Root OWNERS file -- kubebuilder-admins" |
0 commit comments