summaryrefslogtreecommitdiff
path: root/doc/development/code_review.md
diff options
context:
space:
mode:
authorRobert Speicher <rspeicher@gmail.com>2021-01-20 13:34:23 -0600
committerRobert Speicher <rspeicher@gmail.com>2021-01-20 13:34:23 -0600
commit6438df3a1e0fb944485cebf07976160184697d72 (patch)
tree00b09bfd170e77ae9391b1a2f5a93ef6839f2597 /doc/development/code_review.md
parent42bcd54d971da7ef2854b896a7b34f4ef8601067 (diff)
downloadgitlab-ce-6438df3a1e0fb944485cebf07976160184697d72.tar.gz
Add latest changes from gitlab-org/gitlab@13-8-stable-eev13.8.0-rc42
Diffstat (limited to 'doc/development/code_review.md')
-rw-r--r--doc/development/code_review.md17
1 files changed, 11 insertions, 6 deletions
diff --git a/doc/development/code_review.md b/doc/development/code_review.md
index 00f4cf90481..fe395dc2304 100644
--- a/doc/development/code_review.md
+++ b/doc/development/code_review.md
@@ -24,6 +24,7 @@ uncovered edge cases.
The default approach is to choose a reviewer from your group or team for the first review.
This is only a recommendation and the reviewer may be from a different team.
However, it is recommended to pick someone who is a [domain expert](#domain-experts).
+If your merge request touches more than one domain (for example, Dynamic Analysis and GraphQL), ask for reviews from an expert from each domain.
You can read more about the importance of involving reviewer(s) in the section on the responsibility of the author below.
@@ -69,14 +70,17 @@ It picks reviewers and maintainers from the list at the
[engineering projects](https://about.gitlab.com/handbook/engineering/projects/)
page, with these behaviors:
-1. It doesn't pick people whose [GitLab status](../user/profile/index.md#current-status)
- contains the string 'OOO', or the emoji is `:palm_tree:` or `:beach:`.
+1. It doesn't pick people whose Slack or [GitLab status](../user/profile/index.md#current-status):
+ - contains the string 'OOO', 'PTO', 'Parental Leave', or 'Friends and Family'
+ - emoji is `:palm_tree:`, `:beach:`, `:beach_umbrella:`, `:beach_with_umbrella:`, `:ferris_wheel:`, `:thermometer:`, `:face_with_thermometer:`, `:red_circle:`, `:bulb:`, `:sun_with_face:`.
1. [Trainee maintainers](https://about.gitlab.com/handbook/engineering/workflow/code-review/#trainee-maintainer)
are three times as likely to be picked as other reviewers.
-1. People whose [GitLab status](../user/profile/index.md#current-status) emoji
- is `:large_blue_circle:` are more likely to be picked. This applies to both reviewers and trainee maintainers.
+1. Team members whose Slack or [GitLab status](../user/profile/index.md#current-status) emoji
+ is 🔵 `:large_blue_circle:` are more likely to be picked. This applies to both reviewers and trainee maintainers.
- Reviewers with `:large_blue_circle:` are two times as likely to be picked as other reviewers.
- Trainee maintainers with `:large_blue_circle:` are four times as likely to be picked as other reviewers.
+1. People whose [GitLab status](../user/profile/index.md#current-status) emoji
+ is 🔶 `:large_orange_diamond:` are half as likely to be picked. This applies to both reviewers and trainee maintainers.
1. It always picks the same reviewers and maintainers for the same
branch name (unless their OOO status changes, as in point 1). It
removes leading `ce-` and `ee-`, and trailing `-ce` and `-ee`, so
@@ -116,7 +120,7 @@ with [domain expertise](#domain-experts).
by a [Software Engineer in Test](https://about.gitlab.com/handbook/engineering/quality/#individual-contributors)**.
1. If your merge request only includes end-to-end changes (*3*) **or** if the MR author is a [Software Engineer in Test](https://about.gitlab.com/handbook/engineering/quality/#individual-contributors), it must be **approved by a [Quality maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab_maintainers_qa)**
1. If your merge request includes a new or updated [application limit](https://about.gitlab.com/handbook/product/product-processes/#introducing-application-limits), it must be **approved by a [product manager](https://about.gitlab.com/company/team/)**.
-1. If your merge request includes Product Analytics (telemetry) changes, it should be reviewed and approved by a [Product analytics engineer](https://gitlab.com/gitlab-org/growth/product-analytics/engineers).
+1. If your merge request includes Product Intelligence (telemetry or analytics) changes, it should be reviewed and approved by a [Product Intelligence engineer](https://gitlab.com/gitlab-org/growth/product_intelligence/engineers).
- (*1*): Please note that specs other than JavaScript specs are considered backend code.
- (*2*): We encourage you to seek guidance from a database maintainer if your merge
@@ -336,6 +340,7 @@ experience, refactors the existing code). Then:
convey your intent.
- For non-mandatory suggestions, decorate with (non-blocking) so the author knows they can
optionally resolve within the merge request or follow-up at a later stage.
+ - There's a [Chrome/Firefox addon](https://gitlab.com/conventionalcomments/conventional-comments-button) which you can use to apply [Conventional Comment](https://conventionalcomments.org/) prefixes.
- After a round of line notes, it can be helpful to post a summary note such as
"Looks good to me", or "Just a couple things to address."
- Assign the merge request to the author if changes are required following your
@@ -505,7 +510,7 @@ 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
+the 🔴 `:red_circle:` emoji and mentioning that you are at capacity in the status
text. This guides contributors to pick a different reviewer, helping us to
meet the SLO.