summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Thomas <nick@gitlab.com>2019-04-17 10:14:35 +0000
committerNick Thomas <nick@gitlab.com>2019-04-17 10:14:35 +0000
commit128f91fce7a62c7f6c444e670a00294ce1b777a1 (patch)
tree4ea910235ddf34e35322f9dde320d092fd0644a4
parent2811ace5738cae31f766aae41020ecd3332617c3 (diff)
parent95dbc34e29876b9c6f2eaecfd45f9c4ac9824ca7 (diff)
downloadgitlab-ce-128f91fce7a62c7f6c444e670a00294ce1b777a1.tar.gz
Merge branch 'dm-code-review-sla' into 'master'
Rewrite "Review turnaround time" section in light of Global Optimization value See merge request gitlab-org/gitlab-ce!27411
-rw-r--r--doc/development/code_review.md39
1 files changed, 31 insertions, 8 deletions
diff --git a/doc/development/code_review.md b/doc/development/code_review.md
index b1a32e0ed26..4e2213f7742 100644
--- a/doc/development/code_review.md
+++ b/doc/development/code_review.md
@@ -132,14 +132,6 @@ If a developer who happens to also be a maintainer was involved in a merge reque
as a domain expert and/or reviewer, it is recommended that they are not also picked
as the maintainer to ultimately approve and merge it.
-Try to review in a timely manner; doing so allows everyone involved in the merge
-request to iterate faster as the context is fresh in memory. Further, this
-improves contributors' experiences significantly. Reviewers should aim to review
-within two working days from the date they were assigned the merge request. If
-you don't think you'll be able to review a merge request within that time, let
-the author know as soon as possible. When the author of the merge request has not
-heard anything after two days, a new reviewer should be assigned.
-
Maintainers should check before merging if the merge request is approved by the
required approvers.
@@ -220,6 +212,37 @@ It is responsibility of the author of a merge request that the merge request is
Developers who have capacity can regularly check the list of [merge requests to review](https://gitlab.com/groups/gitlab-org/-/merge_requests?scope=all&utf8=%E2%9C%93&state=opened&label_name%5B%5D=ready%20for%20review) and assign any merge request they want to review.
+### Review turnaround time
+
+Since [unblocking others is always a top priority](https://about.gitlab.com/handbook/values/#global-optimization),
+reviewers are expected to review assigned merge requests in a timely manner,
+even when this may negatively impact their other tasks and priorities.
+Doing so allows everyone involved in the merge request to iterate faster as the
+context is fresh in memory, improves contributors' experiences significantly,
+and gives authors more time to address feedback and iterate on their work before
+the [feature freeze](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/PROCESS.md#feature-freeze-on-the-7th-for-the-release-on-the-22nd).
+
+A turnaround time of two working days is usually acceptable, since engineers
+will typically have other things to work on while they're waiting for review,
+but don't hesitate to ask the author if it's unclear what time frame would be
+acceptable, how urgent the review is, or how significant the blockage. Authors
+are also encouraged to provide this information up-front to reviewers, but are
+expected to be mindful of the [guidelines on when to ask for review on MRs that
+are intended to go in before the feature freeze](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/PROCESS.md#between-the-1st-and-the-7th),
+and realistic in their expectations if these were not followed.
+
+If you don't think you'll be able to review a merge request within a reasonable
+time frame, let the author know as soon as possible and try to help them find
+another reviewer or maintainer who will be able to, so that they can be unblocked
+and get on with their work quickly. Of course, if you are out of office and have
+[communicated](https://about.gitlab.com/handbook/paid-time-off/#communicating-your-time-off)
+this through your GitLab.com Status, authors are expected to realize this and
+find a different reviewer themselves.
+
+When a merge request author feels like they have been blocked for longer than
+is reasonable, they are free to remind the reviewer through Slack or assign
+another reviewer.
+
### Reviewing code
Understand why the change is necessary (fixes a bug, improves the user