WIP: Introduce domain maintainers into code review process
What does this MR do?
Introduces the concept of domain maintainers into the code review process. For context, refer to gitlab-com/www-gitlab-com#4517.
Why this change
The GitLab codebase has grown in size and complexity to the point where it is very difficult for a single team member to have context and expertise across the full product.
In line with our objective of availability over velocity we need to ensure that solutions we implement are architecturally sound, performant and secure. To this extend we are introducing the concept of a domain maintainer.
This change aims to to achieve the following objectives:
- Improve the code quality from a solution point of view
- Contribute to code reviews being more efficient
By having team members with in-depth knowledge of a specific area of the codebase and product review all solutions in that area we aim to increase the quality and reduce the amount of time needed to conduct reviews.
By also removing the responsibility for reviewing the solution architecture from codebase maintainers, it allows them to have a more focused task which should further improve the code review process efficiency.
Proposal
This change introduces the concept of a domain maintainer
into our code review process.
To differentiate domain maintainers
from our current maintainers we will refer to them as codebase maintainers
going forward.
Codebase maintainers
are there to ensure that we write code across a single codebase in a coherent (what mechanisms we use) and cohesive (how we write code) way. Our codebase is growing larger and more complex by the day and codebase maintainers
fulfill the critical function of ensuring we keep the barrier to contributing to the codebase low for new team members and community members.
Domain maintainers
are primarily focused on validating the solution being implemented by ensuring that:
- it is accurate
- it is maintainable
- it is consistent with the architecture of the domain area
- it doesn't degrade our application performance or security
Segmenting by stage
Domain scope is defined by the stage that the feature/bug fix belongs to that is being implemented. For automation purposes (i.e. reviewer roulette), the stage can be determined by the devops::xxx
label on the related issue for the MR. If the MR doesn't have an issue associated with it, it is up to the author to identify the stage and choose a relevant domain maintainer.
Code review process amendment
All Merge Requests must be approved by a domain maintainer before being assigned to a codebase maintainer.
The codebase maintainers will merge the code once all relevant approvals have been received.
The codebase maintainer will act as a gatekeeper for ensuring that a domain maintainer has signed off on a merge request before they merge it.
What happens to normal reviews?
Reviews from team members who are not a domain maintainer
or codebase maintainer
are encouraged, but are not mandatory for a merge request to be merged.
Becoming a domain maintainer
The objective is for all team members to be a domain maintainer in the stage they work in.
You can also be a domain maintainer
and codebase maintainer
at the same time, or only 1 of them, or none of them.
Eligibility criteria
- Should be a current team member of the stage (or group in the stage) they represent for at least 3 months.
- Must have a proven record of delivering and/or reviewing MRs in the stage they represent that shows they can fulfill the responsibilities of being a
domain maintainer
.
Nominating new domain maintainers
It is the responsibility of existing domain maintainers
to recommend a team member to become a domain maintainer once the meet the eligibility criteria. Once a team member is recommended it requires the approval of two other domain maintainers
(preferably from same stage) to accept the recommendation.
The DRI for ensuring a healthy domain maintainer
to team member ratio lies with the Director of the stage. A healthy ratio is considered to be 1 domain maintainer
for every 3 team members, with a minimum of 1 domain maintainer
per group.
Defining the initial list of domain maintainers
The Development directors will be responsible for identifying and approving an initial list of team members to become domain maintainers
.
Transition period
To accommodate the change in process and allow time to get all other related matters in place (see follow up actions) we should allow a transition period of 1 milestone. This means that we should only start enforcing this new process from Milestone 12.9 (2020-02-23) onwards.
Unresolved issues
- By introducing a
domain maintainer
review we are breaking the currenttrainee maintainer
(to becometrainee codebase maintainer
) process. Currently trainee maintainers are 3x more likely to be picked by Reviewer Roulette to conduct reviews, however since acodebase maintainer
does not qualify as adomain maintainer
they will no longer be recommended for reviews by the Reviewer Roulette
Further Considerations
- Are the terms
domain maintainer
andcodebase maintainer
clear enough to comply with our MECEFU principle? Again we can debate an update this during the transition period, but should ideally be finalized before the rollout. - Should we consider allowing
domain maintainers
to merge a MR that meets specific criteria themselves without the need for it to go through acodebase maintainer
? Possible examples are minor fixes to tests, copy, docs, or even small enough changes say less than n number of lines etc. This could also be addressed by a follow-up MR. - Should we consider allowing team members who are both a
domain maintainer
and acodebase maintainer
to merge after 1 review? - Segmentation of the codebase by stage: is it feasible, or even preferable, to utilize the CODEOWNERS, functionality to identify areas of the codebase to a stage?
- Is segmentation by stage too big and should we rather segment by group?
Follow up actions
- Update all references to maintainers in the GitLab documentation and Handbook
- Update code review process in the handbook: https://about.gitlab.com/handbook/engineering/workflow/code-review/
- Development Directors should identify, in conjunction with engineering managers, the initial list of domain maintainers for the stages
- Modify reviewer roulette to make domain maintainer recommendation
- Add domain experts to Engineering Projects: https://about.gitlab.com/handbook/engineering/projects/
- Communicate the change to the company