Skip to content

Default to choosing domain experts for reviewers

Jean du Plessis requested to merge jduplessis_domain_experts into master

What does this MR do?

Specifies that choosing of a domain expert should be the default approach when choosing a reviewer.

Why?

There's been discussion for months at GitLab about the need for introducing domain experts into the code review process. Refer to the original issue here: gitlab-com/www-gitlab-com#4517

An original proposal failed for various reasons as outlined here: !23685 (comment 278679950)

We went back to the drawing board where a few team members brainstormed how to proceed: https://docs.google.com/document/d/12lMzdkZNKoie4DuCic4nNBJiLO8gYrCWwgynWPGgqJM/edit#

In the end we decided to run a limit trial with a slightly revised code review process and the outcome was positive: gitlab-com/www-gitlab-com#6584 (comment 315476771)

Insights were also gathered from the Engineering function with regards to the code review process: https://docs.google.com/presentation/d/1ss04ZLa3fEI-JYzv3eu4cDBWooxRB6djdkLE3CwmK4M/edit#slide=id.g6f112ced0f_0_176

Taking all of the above sources into consideration brings us to the proposed process changes in this MR

How?

The key changes to the process can be summarized as:

  1. By default assign MRs to another team member in your own team/group/stage for review
  2. By default assign MRs to maintainers with domain expertise relating to the MR
  3. Maintainers may request an additional review from a domain expert if they feel it is warranted

NB: Inline with the spirit of the current version of the code review process the changes are a recommendation, a default to follow in the absence of a better approach. Team members are still encouraged to use their own discretion to deviate from it where it makes sense and teams can continue setting up their own specific version that aligns with the intent of the process.

What makes a domain expert?

Instead of providing rigid rules for what qualifies you as a domain expert we will use a boring solution of sensible defaults and self-identification.

Sensible defaults:

  • Team members working in a specific stage/group (e.g. create: source code) are considered domain experts for that area of the app they work on
  • Team members working on a specific feature (e.g. search) are considered domain experts for that feature

Self-identification:

  • Team members can self-identify as a domain expert for a specific feature (e.g. file uploads)
  • Team members can self-identify as a domain expert for a specific technology (e.g. GraphQL), product feature (e.g. file uploads) or area of the codebase (e.g. CI).

How to self-identify as a domain expert

The only requirement to be considered a domain expert is to have substantial experience with a specific technology, product feature or area of the codebase. We leave it up to the team member to decide whether they meet this criteria.

We will continue using the current expertise: property in the team.yml to indicate self-identified expertise.

But what if someone identifies as a domain expert and they are not?

As with anything at GitLab a decision can be challenged and reverted, but if we trust employees to spend company money without strict supervision, I believe we can trust team members to judge and identify what they are an expert in.

Where can we find a list of people with domain experts?

If this proposal is accepted we will update the current Engineering Projects page to indicate the expertise of each listed team member.

A follow-up iteration will focus on looking to integrate this information into the Reviewer Roulette.

Follow up actions

  • List the domain expertise of team members on the Engineering Projects page
  • Integrate domain experts recommendations into Reviewer Roulette
  • Update handbook page on Code Review to incorporate information like What makes a domain expert? etc
  • Address the impact this process change has on the trainee maintainer process
Edited by 🤖 GitLab Bot 🤖

Merge request reports