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.

Additional documentation:

• Making a merge request
• 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

List of merge requests for a single project.

Where your attention has been requested:

• Click the "Merge requests" icon at the top of the sidebar to search for requests where you're the assignee.
  • You can filter search results by author, reviewer, approver, milestone, label, reaction, draft status, and additional free text.
• Click the "To-Do list" icon at the top of the sidebar and refine with a type of "Merge Request" and an action of "Assigned", "Review requested", or "Mentioned".
  • You can further filter the list by group, project, and author.

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

A merge request.

When you open a merge request, you'll be presented with an 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

• A reviewer must have explicitly approved the merge request.
• 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. See Squashing for more details.
  • 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.
• CI pipelines must pass.