diff --git a/CODE-OWNERS b/CODE-OWNERS new file mode 100644 index 000000000000..05551be332bf --- /dev/null +++ b/CODE-OWNERS @@ -0,0 +1,37 @@ +# Code ownership file that specifies approvers and reviewers for all areas of the Rook codebase. +# all: These members have the given role for all areas of the codebase. +# common: The rook.io common types, specs, logic, etc. +# build: The top level makefile, as well as the scripts and makefiles in the `build` directory. +# All other areas are for specific storage providers and covers all portions of code that is specific to their project. + +areas: + all: + approvers: + - travisn + - galexrt + - jbw976 + common: + approvers: + - bassam + build: + approvers: + - bassam + ceph: + approvers: + - BlaineEXE + - leseb + reviewers: + - noahdesu + edgefs: + approvers: + - dyusupov + reviewers: + - sabbot + cassandra: + approvers: + - yanniszark + nfs: + reviewers: + - rohan47 + cockroachdb: + minio: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1acec13c532d..0c7ee2d907dd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -105,3 +105,75 @@ The first line is the subject and should be no longer than 70 characters, the second line is always blank, and other lines should be wrapped at 80 characters. This allows the message to be easier to read on GitHub as well as in various git tools. + +## Change Approval + +The Rook project aims to empower contributors to approve and merge code changes autonomously. +The maintainer team does not have sufficient resources to fully review and approve all proposed code changes, so trusted members of the community are given these abilities according to the process described in this section. +The goal of this process is to increase the code velocity of all storage providers and streamline their day to day operations such as pull request approval and merging. + +### Change Approval Roles + +The model for approving changes is largely based on the [Kubernetes code review process](https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#code-review-using-owners-files), +where a set of roles are defined for different portions of the code base and have different responsibilities: + +* **Reviewers** are able to review code for quality and correctness on some part of the project, but cannot merge changes. +* **Approvers** are able to both review and approve code contributions. While code review is focused on code quality and correctness, approval is focused on holistic acceptance of a contribution. Approvers can merge changes. + +Both of these roles will require a time commitment to the project in order to keep the change approval process moving forward at a reasonable pace. +When automation is implemented to auto assign members to review pull requests, it will be done in a round-robin fashion, so all members must be able to dedicate the time needed. + +Note that neither of these roles have voting powers in conflict resolution, these roles are for the code base change approval process only. + +### Pull Request Flow + +The general flow for a pull request approval process is as follows: + +1. Author submits the pull request +1. Reviewers and approvers for the applicable code areas review the pull request and provide feedback that the author integrates +1. Reviewers and/or approvers signify their LGTM on the pull request +1. An approver approves the pull request based on at least one LGTM from the previous step + 1. Note that the approver can heavily lean on the reviewer for examining the pull request at a finely grained detailed level. The reviewers are trusted members and approvers can leverage their efforts to reduce their own review burden. +1. An approver merges the pull request into the target branch (master, release, etc.) + +### Role Assignments + +#### Declarations + +All roles will be assigned by the usage of [`CODE-OWNERS`](CODE-OWNERS) files committed to the code base. +These assignments will be initially be defined in a single file at the root of the repo and it will describe all assigned roles for the entire code base. +As we incorporate automation (i.e. bots) into this change acceptance process in the future, we can reorganize this initial single owners file into separate files amongst the codebase as the automation necessitates. + +The format of the file can start with simply listing the reviewers and approvers for areas of the code base using a YAML format: + +```yaml +areas: + feature-foo: + approvers: + - alice + - bob + reviewers: + - carol +``` + +#### Update Process + +The process for adding or removing reviewers/approvers is described in the [project governance](GOVERNANCE.md#updating-change-approval-roles). + +### Permissions + +Role assignees will be made part of the following Rook organization teams with the given permissions: + +* **Reviewers:** added to a new Reviewers team so they have write permissions to the repo to assign issues, add labels to issues, add issues to milestones and projects, etc. but cannot merge to protected branches such as `master` and `release-*`. +* **Approvers:** added to a new Approvers team that will have access to merge to protected branches. + +### Automation + +This process can be further improved by automation and bots to automatically assign the PR to reviewers/approvers, add labels to the PR, and merge the PR. +We should explore this further with some experimentation and potentially leveraging what Kubernetes has done, but automation isn’t strictly required to adopt and implement this model. + +### Alternatives Considered + +The built in support in Github for [`CODEOWNERS`](https://help.github.com/en/articles/about-code-owners) files was considered. +However, this only supports the automated assignment of reviewers to pull requests. +It has no tiering or differentiation between roles like the proposed approvers/reviewers model has and is therefore not a good fit. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 80a5dc32d9d0..78ecf1e85fc8 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -2,7 +2,7 @@ This document defines governance policies for the Rook project. -## Roles +## Maintainer Roles Rook uses a two-tiered system of maintainer roles: @@ -95,15 +95,6 @@ the maintainers per the voting process below. * In general continue to be willing to spend at least 25% of ones time working on Rook (~1.25 business days per week). -### Approving PRs - -PRs may be merged after receiving at least **1 approval from a maintainer** (either senior or standard) -that is **not the author** of the PR, and preferably from a **different organization** than the PR author. -As complexity of a PR increases, such as design changes or major PRs, the need for an approval from -a different organization also increases. This should be a judgement call from the maintainers, -and it is expected that all maintainers act in good faith to seek approval from a different -organization when appropriate. - ### Github Project Administration Maintainers will be added to the Rook GitHub organization (if they are not already) and added to @@ -111,6 +102,16 @@ the GitHub Maintainers team. After 6 months, **senior** maintainers will be made an "owner" of the Rook GitHub organization. +## Updating Change Approval Roles + +The full change approval process is described in the [contributing guide](CONTRIBUTING.md#change-approval). + +All new reviewers and approvers must be nominated by someone (anyone) opening a pull request that adds the nominated person’s name to the appropriate [`CODE-OWNERS`](CODE-OWNERS) files in the appropriate roles. +Similarly, to remove a reviewer or approver, a pull request should be opened that removes their name from the appropriate [`CODE-OWNERS`](CODE-OWNERS) files. + +The maintainer team will approve this update in the standard voting and conflict resolution process. +Note that new reviewer/approver nominations do not go through the standard pull request approval described in the [contributing guide](CONTRIBUTING.md#change-approval), only the maintainer team can approve updates of members to the change approval roles. + ## Conflict resolution and voting As previously mentioned, senior maintainers receive **2 votes** each and standard maintainers diff --git a/OWNERS.md b/OWNERS.md index 262a4410695b..770d47487e95 100644 --- a/OWNERS.md +++ b/OWNERS.md @@ -25,6 +25,10 @@ Maintainers will be added according to the [process for adding a maintainer](GOV There are currently no emeritus maintainers. This list will be updated according to the [process for removing a maintainer](GOVERNANCE.md#removing-a-maintainer) in the [governance](GOVERNANCE.md). +## Code Owners + +Code owners, as described in the [change approval process](GOVERNANCE.md#change-approval) of the governance, can be found in [`CODE-OWNERS`](CODE-OWNERS) files. + ## Friends of Rook This section lists people that are not currently maintainers but have demonstrated value to the community.