Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorToon Claes <toon@gitlab.com>2021-02-08 19:25:30 +0300
committerToon Claes <toon@gitlab.com>2021-02-10 18:54:36 +0300
commit56136e0beb12a59391b0b311be7df33244d47cb5 (patch)
treeec614d1fb1c9bd5459be78deffa230639618bdf3 /REVIEWING.md
parentb31a58deedc74c9471de32c13b21ef1c1e247485 (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.md35
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