Skip to content

Update Code Review process to use Reviewers

Michelle Gill requested to merge mgill/dogfooding-reviewers into master

What does this MR do?

We are updating the code review process based on the positive feedback that we received from maintainers in the proposal. This MR makes a few minor but key changes:

  1. Authors and/or DRI's remain the assignee
  2. Reviewers and Maintainers get added as "Reviewers" in the sidebar
  3. Reviewers and maintainers who are not merging will unassign themselves as a reviewer after each review given

We understand that this was not the most popular option overall, and in the future we hope to keep all reviewers in the sidebar permanently, but we determined that this was the best iteration based on what we have today and where we want to go in the future. This option provides the highest consistency today and prevents us from:

  1. checking both MRs assigned and Review Requests to find where we need to take action
  2. disregarding the reviewer feature
  3. reminding people of our personal preferences ... with the benefit of dogfooding what we hope will be a highly used feature!

For more information on what we wanted to accomplish with this process, the feedback we received, and the alternative options we considered, view this presentation.

Link to Deploy App

Does this MR meet the acceptance criteria?

Conformity

Availability 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 André Luís

Merge request reports