Code Review

1. You are here:
2. Handbook
3. Engineering
4. Engineering Workflow
5. Code Review

On this page

• Overview
• Reviewer
• Maintainer
  • How to become a maintainer
    • Trainee maintainer
    • Self-directed

Overview

Code reviews are mandatory for every merge request, you should get familiar and follow our Code Review Guidelines.

These guidelines also describe who would need to review, approve and merge your merge request.

Reviewer

All GitLab engineers can, and are encouraged to, perform code review on merge requests of colleagues and community contributors. If you want to review merge requests, you can wait until someone assigns you one, but you are also more than welcome to browse the list of open merge requests and leave any feedback or questions you may have.

To let other engineers know that you are interested in performing code reviews, you can add this to your team page entry by editing data/team.yml.

You can find someone to review your own merge requests by looking on the team page, or on the list of GitLab Engineering Projects, both of which are fed by data/team.yml.

You can also help community contributors get their merge requests ready, by becoming a Merge Request Coach.

Note that while all engineers can review all merge requests, the ability to accept merge requests is restricted to maintainers.

Maintainer

Maintainers are GitLab engineers who are experts at code review, know the GitLab product and code base very well, and are empowered to accept merge requests in one or several GitLab Engineering Projects.

Every project has at least one maintainer, but most have multiple, and some projects (like GitLab CE and GitLab EE) have separate maintainers for frontend and backend.

Great engineers are often also great reviewers, but code review is a skill in and of itself, and not every engineer, no matter their seniority, will have had the same opportunities to hone that skill. It's also important to note that a big part of being a good maintainer comes from knowing the existing product and code base extremely well, which lets them spot inconsistencies, edge cases, or non-obvious interactions with other features that would otherwise be missed easily.

To protect and ensure the quality of the code base and the product as a whole, people become maintainers only once they have convincingly demonstrated that their reviewing skills are at a comparable level to those of existing maintainers.

As with regular reviewers, maintainers can be found on the team page, or on the list of GitLab Engineering Projects.

How to become a maintainer

This applies specifically to backend and frontend maintainers. Other areas (frontend, database, etc.) may have separate processes.

As a reviewer, a great way to improve your reviewing skills is to participate in MRs. Add your review notes, pass them on to maintainers, and follow the conversation until the MR is closed. If a comment doesn't make sense to you, ask the commenter to explain further. If you missed something in your review, figure out why you didn't see it, and note it down for next time.

We have two guidelines for maintainership, but no concrete rules:

1. Junior engineers should be focused on becoming intermediate engineers over attempting to become maintainers.
   1. In general, the further along in their career someone is, the more we expect them to be capable of becoming a maintainer.
2. Most people should aim for being at GitLab for a year before applying to be a maintainer. This is to get a good feel for the codebase, expertise in one or more domains, and deep understanding of our coding standards. For Staff levels and higher, this may be reduced.

Apart from that, someone can be considered as a maintainer when both:

1. The MRs they've reviewed consistently make it through maintainer review without significant additionally required changes.
2. The MRs they've written consistently make it through reviewer and maintainer review without significant required changes.

Once those are done, they should:

1. Create an MR to add the maintainership to their team page entry.
2. Explain in the MR body why they are ready to take on that responsibility.
3. Use specific examples of recent "maintainer-level" reviews that they have performed.
   1. The MRs should not reflect only small changes to the code base, but also architectural ones and ones that create a full feature.
4. Assign the MR to their manager and mention the existing maintainers of the relevant product (GitLab CE, GitLab EE, etc) and area (backend, frontend, etc.).

If the existing maintainers of the relevant engineering group e.g., backend, do not have significant objections, and if at least half of them agree that the reviewer is indeed ready, we've got ourselves a new maintainer!

The existing maintainers of the relevant engineering group will also raise any areas for growth on the merge request. If there are many gaps, the reviewer will need to address these before asking for reconsideration.

Trainee maintainer

In order to help grow the maintainer base with the team, we allow for 'trainee maintainers'. These are reviewers who have shown a specific interest in becoming a maintainer, and are actively working towards that goal.

Being a trainee maintainer means:

1. Having an assigned buddy maintainer to give direct, specific feedback about your reviews.
2. Reviewing more merge requests - your buddy may assign you non-time-critical MRs to review, to give more opportunities for review.

We use a tracking issue for each maintainer / trainee pair, using the Trainee backend maintainer template.

Self-directed

It is not necessary to go through the trainee maintainer stage to become a maintainer. Reviewers can make the case for their own maintainership status, as described above.

If you'd like to work towards becoming a maintainer, discuss it in your regular 1:1 meetings with your manager. They will help you to identify areas to work on before following the process above.