diff options
author | Toon Claes <toon@gitlab.com> | 2021-02-08 19:25:30 +0300 |
---|---|---|
committer | Toon Claes <toon@gitlab.com> | 2021-02-10 18:54:36 +0300 |
commit | 56136e0beb12a59391b0b311be7df33244d47cb5 (patch) | |
tree | ec614d1fb1c9bd5459be78deffa230639618bdf3 /REVIEWING.md | |
parent | b31a58deedc74c9471de32c13b21ef1c1e247485 (diff) |
Add a note about selecting reviewers
It was not clear from the documentation it is recommended to follow
Danger bot to select the two reviewers suggested.
This change adds that guideline and elaborates a bit on the roles of
contributor and reviewer.
Diffstat (limited to 'REVIEWING.md')
-rw-r--r-- | REVIEWING.md | 35 |
1 files changed, 28 insertions, 7 deletions
diff --git a/REVIEWING.md b/REVIEWING.md index 9b23711a3..bc1b9a5d7 100644 --- a/REVIEWING.md +++ b/REVIEWING.md @@ -1,16 +1,30 @@ # Gitaly code review process -## Tips for streamlined and thorough reviews +## Roles -Goals of these tips: +**contributor**: usually the creator of the merge request. They are the +[DRI](https://about.gitlab.com/handbook/people-group/directly-responsible-individuals/) +responsible for getting the MR to a merge state. -1. Streamline the review and acceptance process: improve throughput -2. Ensure a thorough review: minimize the number of problems that are discovered after merging +**reviewer**: the person reviewing the merge request and looking out the +guidelines in this document are followed. Each MR should be approved by at least +one reviewer. + +### Choosing reviewers + +When a merge request is created, Danger bot will suggest two people for +review. To spread the load across the team, it's generally recommended to assign +these two for review. But it's not uncommon to swap out one or two of them when: +- they are OOO +- they know the context of the change already + +The complete list of elegible reviewers can be found at: +https://about.gitlab.com/handbook/engineering/projects/#gitaly_assignments -### Roles +For small changes it's fine to only chose one person for review. The contributor +can ask the reviewer if they are okay with this. -There is one **contributor**: the person who owns the MR and is trying to get it -merged. There is at least one **reviewer**. +## Criteria The main review criteria are: @@ -22,6 +36,13 @@ The last point is easy to overlook. For example, you don't want to merge some crystal clear water-tight piece of code that causes a production outage, because causing an outage is not the right thing to do. +## Tips for streamlined and thorough reviews + +Goals of these tips: + +1. Streamline the review and acceptance process: improve throughput +2. Ensure a thorough review: minimize the number of problems that are discovered after merging + ### Tips for the Contributor As the contributor you have a dual role. You are driving the change in the MR, but you |