Merge remote-tracking branch 'upstream/master' into 19737-read-only-auditor19737-read-only-auditor

* upstream/master: (1277 commits)
  Grapify the labels API
  Fix typo in project settings that prevents users from enabling container registry.
  Fix old monitoring links to point to the new location
  Added path parameter to Commits API
  fixes build with cache:clear issue
  Merge branch 'security-fix-leaking-namespace-name' into 'security'
  Fix authored vote from notes
  Grapify builds API
  Add changelog item for groups 404 on relative url
  Add relative url support to routing contrainers
  Update project member controller to match recent master logic
  Add parentheses around return redirect_to method
  Trigger change even in select2 test helper to produce production-like behaviour
  Refactor js that disable form submit if no members selected
  Improve create project member test at project_members_controller_spec
  Move changelog item to 8.14
  Refactor create member tests from group_members_controller_spec
  Refactor groups/projects members controller
  Gracefully handle adding of no users to projects and groups
  Revert "Change "Group#web_url" to return "/groups/twitter" rather than "/twitter"."
  ...

1 files changed, 9 insertions, 2 deletions

diff --git a/doc/development/code_review.md b/doc/development/code_review.md
index 40ae55ab905..c5c23b5c0b8 100644
--- a/doc/development/code_review.md
+++ b/doc/development/code_review.md
@@ -34,6 +34,10 @@ request is up to one of our merge request "endbosses", denoted on the
 
 ## Having your code reviewed
 
+Please keep in mind that code review is a process that can take multiple
+iterations, and reviewers may spot things later that they may not have seen the
+first time.
+
 - The first reviewer of your code is _you_. Before you perform that first push
   of your shiny new branch, read through the entire diff. Does it make sense?
   Did you include something unrelated to the overall purpose of the changes? Did
@@ -55,6 +59,7 @@ request is up to one of our merge request "endbosses", denoted on the
 Understand why the change is necessary (fixes a bug, improves the user
 experience, refactors the existing code). Then:
 
+- Try to be thorough in your reviews to reduce the number of iterations.
 - Communicate which ideas you feel strongly about and those you don't.
 - Identify ways to simplify the code while still solving the problem.
 - Offer alternative implementations, but assume the author already considered
@@ -64,8 +69,10 @@ experience, refactors the existing code). Then:
   someone else would be confused by it as well.
 - 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 ("Merge when build
-  succeeds" is fine).
+- 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
+  subsequent revisions for anything that would be spotted after that.
 
 ## Credits