Skip to content

Choosing a reviewer from a team member in order to gain the team knowledge

Shinya Maeda requested to merge docs-ask-review-from-proper-team into master

Problem

Basically, all merge requests must be reviewed by two types of engineers, 1. Reviewer 2. Maintainer. Our current reviewing guideline states that "You can choose a reviewer from a different team" at the first place, however, this might not be ideal from a team-work PoV.

Say, you're one of the Release engineers and you ask someone from Create team to review your merge request, the Create team member will know that what change will be shipped in the current milestone, how the feature was added/modified from the previous version, etc - through the reviewing process. All the knowledge will go to the Create team instead of Release team, even though Release team has a responsibility to develop and maintain these features. This leads that the other Release team members will have no idea what's going on the other team member's changes.

Ideally, these knowledge should go to Release team at the reviewing phase.

The other advantages I can think of

  • This will let us have more communications in the team, which could result in the more tight teamwork.
  • More thoughtful review
  • Easy to track changes in team

Proposal

Here is my proposal to find a relevant reviewer for your merge request. From top to bottom, you can go through until you find the one.

  1. If the merge request has a team label, choose someone from a corresponding team to review. (e.g. https://about.gitlab.com/handbook/engineering/ops/release/)
  2. Choose one from our dedicated reviewers

Does this MR meet the acceptance criteria?

Conformity

Performance and testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Shinya Maeda

Merge request reports