Code review#

This page documents code review practices used for Software Heritage development.

GitLab has many useful features to ease reviewing the changes, communicating around questions or comments, and making suggestions. Learning how to use them will help you and the team.

Guidelines#

Please adhere to the following guidelines in the context of Software Heritage development.

When submitting changes:

  • Reviews are strongly recommended for any non-trivial code change, but not mandatory (nor enforced at Git level).

  • The code submission workflow is implemented using merge requests sent to our GitLab instance.

  • The assignee in the context of merge request is the person supposed to drive it to completion. In almost all cases, you should assign yourself to merge requests you create.

  • If your changes are meant to address a specific issue, you should prefix the branch name with the issue number.

  • Feel free to explicitly mention one or more specific reviewer from people most knowledgeable with the target code. You can also assign a single person [1] as reviewer using the relevant field. It will prevent the merge request from going unnoticed.

  • One approval is enough before merging a request.

As a team member:

  • Review any merge requests you want: no matter the suggested reviewers, feel free to review any pending merge requests.

  • Review every day: reviews should be timely as fellow developers will wait for them. To make the process sustainable each developer should strive to dedicate a fixed minimum amount of review time every workday.

Learning about pending reviews#

As we aim to review merge requests in a timely manner, here are several ways to know about merge requests waiting for input.

Notifications#

To be sure to receive notifications of new merge requests:

  1. Open the notification controls for your GitLab account.

  2. Locate the Platform group.

  3. Select Custom in the list of notification levels.

  4. Click on the bell icon.

  5. Make sure New merge request is ticked. You probably also want to select at least New issue, Failed pipeline, and Fixed pipeline.

  6. Click Ok.

  7. Repeat the operation for other groups of interest.

List pending merge requests#

Sadly, notifications can easily be missed and GitLab by itself does not provide a view with all merge requests waiting for action across multiple projects. To have a better view of pending merge requests, you should consider using:

  • GitLab Notify Extension available for Chrome and Firefox. It can list all merge requests you are assigned to and all merge requests you created.

  • gitlab-revq, a command-line tool listing all actionable pending requests.

See also#