GitLab/Workflows/Code review

Code review is an essential part of our contribution workflow. The principle is basic: any patch must be reviewed by others before being merged. Accordingly, we mandate a review-before-merge workflow for any source code we deploy.

See Gerrit to GitLab for pointers on how specific features and tasks translate across systems.

The merge request model
The core abstraction for code review in GitLab is the merge request. The terminology differs, but this is essentially the pull request model first popularized by GitHub: A developer makes a branch, makes one or more commits, and submits a request to merge those changes. Depending on the developer's access level, branches may be pushed either to a copy of the repository forked to their personal account, or directly to the mainline repository.

Once a merge request is submitted, reviewers may comment or make suggestions. In response to feedback, the developer can push more commits to their work branch. Reviewers may then either merge or close the request, as appropriate

Users with the authority to merge a merge request may also push commits to the underlying branch.

Upstream documentation:


 * Merge requests
 * Getting started with Merge Requests

Review workflow
Upstream documentation:


 * Reviewing and managing merge requests
 * Threads

Requesting reviews
GitLab allows you to assign a single person as a reviewer. If you want multiple people to review your merge request, you can leave a comment in the merge request and @mention each user you'd like to review. It's also possible to comment inline, to ask a reviewer to comment on a specific part of the patch.

Finding merge requests to review


Where your attention has been requested:


 * Click the "Merge requests" icon in the upper righthand navigation bar to search for requests where you're the assignee.
 * Click the "To-Do list" icon in the upper righthand navigation bar and refine with a type of "Merge Request" and an action of "Directly Addressed".
 * Search may be refined by author, assignee, draft status, target branch, label, or reaction.
 * Click "More" → "Activity" in the upper lefthand navigation bar, and narrow by type of activity

For a single project:


 * Navigate to the specific project and click "Merge Requests" on the lefthand sidebar

For a group of projects:


 * Navigate to the group and click "Merge Requests" on the lefthand sidebar.

Reviewing code


When you open a merge request, you'll be presented with a overview that includes the merge request title, description (if any), status, assignees, and a history of comments and other actions.

From this overview, click:


 * "Commits" to see a list of commits to the branch.
 * "Changes" to see a view containing a diff of all the changes represented by the merge request.

When viewing changes, you can switch between an inline diff and a side-by-side view using the gear icon on the upper right. Hovering your cursor over individual lines in this view will display a comment button to the left of the line. To comment on multiple lines at once, select the desired lines and click the comment button.

Inline suggestions
GitLab comments support inline suggestions for simple changes. This is a powerful feature, particularly for small, uncontroversial changes such as typo fixes or style tweaks.

As a reviewer, you can add a suggestion using the following syntax (also available by clicking the "Insert suggestion" button on the comment formatting toolbar):

```suggestion:-0+0 replacement text here ```

Best practices:


 * Use inline suggestions for minor changes (typos, style issues, clarifying comments).
 * Multiple suggestions should generally be applied as a batch.
 * A merge request with accepted suggestions should typically be squashed on merge in order to avoid noisy commits.

Upstream documentation: Threads - Suggest changes

Requirements to merge

 * All discussions (aka: "threads") on the merge request must be resolved.
 * History must be clean.
 * This means that the merge request must either:
 * Be set to squash on merge.
 * Be rebased to include only logically grouped commits, so that every commit which becomes part of the mainline branch will make sense on its own, and meaningless commits such as typo fixes are avoided.
 * See Squashing for more detail.
 * CI pipelines must pass.