Merge branch 'docs-merge-results' into 'master'

Update the code review guide to take the Merge Results in account

See merge request gitlab-org/gitlab!25449

doc/development/code_review.md

@@ -89,7 +89,7 @@ that it does so in the most appropriate way, that it satisfies all requirements,
 and that there are no remaining bugs, logical problems, uncovered edge cases,
 or known vulnerabilities. The best way to do this, and to avoid unnecessary
 back-and-forth with reviewers, is to perform a self-review of your own merge
-request, following the [Code Review](#reviewing-code) guidelines.
+request, following the [Code Review](#reviewing-a-merge-request) guidelines.
 
 To reach the required level of confidence in their solution, an author is expected
 to involve other people in the investigation and implementation processes as
@@ -129,7 +129,7 @@ This
 
 ### The responsibility of the reviewer
 
-[Review the merge request](#reviewing-code) thoroughly. When you are confident
+[Review the merge request](#reviewing-a-merge-request) thoroughly. When you are confident
 that it meets all requirements, you should:
 
 - Click the Approve button.
@@ -211,7 +211,7 @@ Instead these should be sent to the [Release Manager](https://about.gitlab.com/c
   mentioning them; this will ensure they see it if their notification level is
   set to "mentioned" and other people will understand they don't have to respond.
 
-### Having your code reviewed
+### Having your merge request 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
@@ -244,52 +244,17 @@ first time.
 
 If you want to have your merge request reviewed, you can assign it to any reviewer. The list of reviewers can be found on [Engineering projects](https://about.gitlab.com/handbook/engineering/projects/) page.
 
-You can also use `ready for review` label. That means that your merge request is ready to be reviewed and any reviewer can pick it. It is recommended to use that label only if there isn't time pressure and make sure the merge request is assigned to a reviewer.
+You can also use `workflow::ready for review` label. That means that your merge request is ready to be reviewed and any reviewer can pick it. It is recommended to use that label only if there isn't time pressure and make sure the merge request is assigned to a reviewer.
 
 When your merge request was reviewed and can be passed to a maintainer you can either pick a specific maintainer or use a label `ready for merge`.
 
 It is responsibility of the author of a merge request that the merge request is reviewed. If it stays in `ready for review` state too long it is recommended to assign it to a specific reviewer.
 
-### List of merge requests ready for review
+#### List of merge requests ready for review
 
-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.
+Developers who have capacity can regularly check the list of [merge requests to review](https://gitlab.com/groups/gitlab-org/-/merge_requests?state=opened&label_name%5B%5D=workflow%3A%3Aready%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, and improves contributors' experience significantly.
-
-#### Review-response SLO
-
-To ensure swift feedback to ready-to-review code, we maintain a `Review-response` Service-level Objective (SLO). The SLO is defined as:
-
-> - review-response SLO = (time when first review response is provided) - (time MR is assigned to reviewer) < 2 business days
-
-If you don't think you'll be able to review a merge request within the `Review-response` SLO
-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.
-
-If you think you are at capacity and are unable to accept any more reviews until
-some have been completed, communicate this through your GitLab status by setting
-the `:red_circle:` emoji and mentioning that you are at capacity in the status
-text. This will guide contributors to pick a different reviewer, helping us to
-meet the SLO.
-
-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 has been blocked for longer than
-the `Review-response` SLO, they are free to remind the reviewer through Slack or assign
-another reviewer.
-
-### Reviewing code
+### Reviewing a merge request
 
 Understand why the change is necessary (fixes a bug, improves the user
 experience, refactors the existing code). Then:
@@ -310,26 +275,47 @@ experience, refactors the existing code). Then:
   "LGTM :thumbsup:", or "Just a couple things to address."
 - Assign the merge request to the author if changes are required following your
   review.
-- Set the milestone before merging a merge request.
-- Ensure the target branch is not too far behind master. If
-[master is red](https://about.gitlab.com/handbook/engineering/workflow/#broken-master),
-it should be no more than 100 commits behind.
+
+### Merging a merge request
+
+Before taking the decision to merge:
+
+- Set the milestone.
 - Consider warnings and errors from danger bot, code quality, and other reports.
-Unless a strong case can be made for the violation, these should be resolved
-before merge.
-- Ensure a passing CI pipeline or if [master is broken](https://about.gitlab.com/handbook/engineering/workflow/#broken-master), post a comment mentioning the failure happens in master with a
-link to the ~"master:broken" issue.
-- Avoid accepting a merge request before the job succeeds. Of course, "Merge
-  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.
+  Unless a strong case can be made for the violation, these should be resolved
+  before merge. A comment must to be posted if the MR is merged with any failed job.
+
+When ready to merge:
+
 - Consider using the [Squash and
-  merge][squash-and-merge] feature when the merge request has a lot of commits.
+  merge](../user/project/merge_requests/squash_and_merge.md#squash-and-merge)
+  feature when the merge request has a lot of commits.
   When merging code a maintainer should only use the squash feature if the
   author has already set this option or if the merge request clearly contains a
   messy commit history that is intended to be squashed.
+- **Start a new merge request pipeline with the `Run Pipeline` button in the merge
+  request's "Pipelines" tab, and enable "Merge When Pipeline Succeeds" (MWPS).** Note that:
+  - If the **latest [Pipeline for Merged Results](../ci/merge_request_pipelines/pipelines_for_merged_results/#pipelines-for-merged-results-premium)** finished less than 2 hours ago, you
+    might merge without starting a new pipeline as the merge request is close
+    enough to `master`.
+  - If the merge request is from a fork, check how far behind `master` the
+    source branch is. If it's more than 100 commits behind, ask the author to
+    rebase it before merging.
+  - If [master is broken](https://about.gitlab.com/handbook/engineering/workflow/#broken-master),
+    in addition to the two above rules, check that any failure also happens
+    in `master` and post a link to the ~"master:broken" issue before clicking the
+    red "Merge" button.
+- When you set the MR to "Merge When Pipeline Succeeds", you should take over
+  subsequent revisions for anything that would be spotted after that.
 
-[squash-and-merge]: ../user/project/merge_requests/squash_and_merge.md#squash-and-merge
+NOTE: **Note:**
+Thanks to "Pipeline for Merged Results", authors won't have to rebase their
+branch as frequently anymore (only when there are conflicts) since the Merge
+Results Pipeline will already incorporate the latest changes from `master`.
+This results in faster review/merge cycles since maintainers don't have to ask
+for a final rebase: instead, they only have to start a MR pipeline and set MWPS.
+This step brings us very close to the actual Merge Trains feature by testing the
+Merge Results against the latest `master` at the time of the pipeline creation.
 
 ### The right balance
 
@@ -411,6 +397,41 @@ Enterprise Edition instance. This has some implications:
 1. **Filesystem access** can be slow, so try to avoid
    [shared files](shared_files.md) when an alternative solution is available.
 
+### 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, and improves contributors' experience significantly.
+
+#### Review-response SLO
+
+To ensure swift feedback to ready-to-review code, we maintain a `Review-response` Service-level Objective (SLO). The SLO is defined as:
+
+> - review-response SLO = (time when first review response is provided) - (time MR is assigned to reviewer) < 2 business days
+
+If you don't think you'll be able to review a merge request within the `Review-response` SLO
+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.
+
+If you think you are at capacity and are unable to accept any more reviews until
+some have been completed, communicate this through your GitLab status by setting
+the `:red_circle:` emoji and mentioning that you are at capacity in the status
+text. This will guide contributors to pick a different reviewer, helping us to
+meet the SLO.
+
+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 has been blocked for longer than
+the `Review-response` SLO, they are free to remind the reviewer through Slack or assign
+another reviewer.
+
 ## Examples
 
 How code reviews are conducted can surprise new contributors. Here are some examples of code reviews that should help to orient you as to what to expect.