Merge branch 'new-docs-topic-issues' of gitlab.com:gitlab-org/gitlab-ce into new-docs-topic-issuesnew-docs-topic-issues

1 files changed, 29 insertions, 3 deletions

diff --git a/doc/development/code_review.md b/doc/development/code_review.md
index c5c23b5c0b8..e4a0e0b92bc 100644
--- a/doc/development/code_review.md
+++ b/doc/development/code_review.md
@@ -9,7 +9,7 @@ code is effective, understandable, and maintainable.
 
 Any developer can, and is encouraged to, perform code review on merge requests
 of colleagues and contributors. However, the final decision to accept a merge
-request is up to one of our merge request "endbosses", denoted on the
+request is up to one the project's maintainers, denoted on the
 [team page](https://about.gitlab.com/team).
 
 ## Everyone
@@ -70,10 +70,36 @@ experience, refactors the existing code). Then:
 - After a round of line notes, it can be helpful to post a summary note such as
   "LGTM :thumbsup:", or "Just a couple things to address."
 - Avoid accepting a merge request before the build succeeds. Of course, "Merge
-  When Build Succeeds" (MWBS) is fine.
-- If you set the MR to "Merge When Build Succeeds", you should take over
+  When Pipeline Succeeds" (MWPS) is fine.
+- If you set the MR to "Merge When Pipeline Succeeds", you should take over
   subsequent revisions for anything that would be spotted after that.
 
+## The right balance
+
+One of the most difficult things during code review is finding the right
+balance in how deep the reviewer can interfere with the code created by a
+reviewee.
+
+- Learning how to find the right balance takes time; that is why we have
+  reviewers that become maintainers after some time spent on reviewing merge
+  requests.
+- Finding bugs and improving code style is important, but thinking about good
+  design is important as well. Building abstractions and good design is what
+  makes it possible to hide complexity and makes future changes easier.
+- Asking the reviewee to change the design sometimes means the complete rewrite
+  of the contributed code. It's usually a good idea to ask another maintainer or
+  reviewer before doing it, but have the courage to do it when you believe it is
+  important.
+- There is a difference in doing things right and doing things right now.
+  Ideally, we should do the former, but in the real world we need the latter as
+  well. A good example is a security fix which should be released as soon as
+  possible. Asking the reviewee to do the major refactoring in the merge
+  request that is an urgent fix should be avoided.
+- Doing things well today is usually better than doing something perfectly
+  tomorrow. Shipping a kludge today is usually worse than doing something well
+  tomorrow. When you are not able to find the right balance, ask other people
+  about their opinion.
+
 ## Credits
 
 Largely based on the [thoughtbot code review guide].