Dev guidelines: How to use GL Reviewers internally
Problem to solve
We're introducing the new Reviewers feature in GitLab:
How will we use this internally for GL development?
https://docs.gitlab.com/ee/development/code_review.html#reviewing-a-merge-request
I've seen a lot of people asking about this on Slack:
- https://gitlab.slack.com/archives/CCB575BGT/p1607382823292900
- https://gitlab.slack.com/archives/CK75EF2A2/p1607384779148000
- https://gitlab.slack.com/archives/C02PF508L/p1607390896492100
- https://gitlab.slack.com/archives/C0140D3ELFN/p1607673287021600
- https://gitlab.slack.com/archives/C0259241C/p1602598781057700
- https://gitlab.slack.com/archives/C01EMBKS5DW/p1607969410317900
- https://gitlab.slack.com/archives/CMJ8JR0RH/p1607948497127500
- https://gitlab.slack.com/archives/CJHPLRTRD/p1607940846294000
- https://gitlab.slack.com/archives/C01EMBKS5DW/p1608005914335700
See also: #292938 (comment 467038750) for reasons & quotes.
Proposal
Define a workflow for the use of Reviewers in GitLab development, so that we can:
- Rely on the use of the feature.
- Use it consistently across the board.
- Use the data we can collect from it.
- At a glance, see who is to review, is reviewing, and has reviewed a merge request.
Suggestion:
- Use "Reviewer" to indicate who the MR was reviewed by. Once added as Reviewer (and actually reviewed the MR), never remove the person from the Reviewers field.
- Use "Assignee" to indicate who is working on the MR at the moment.
Proposal details
For a team member requesting a review
- Reviewers field: Add the reviewer to the Reviewer field.
- Assignee field: Assign the reviewer to the MR.
Then, if the reviewer passes the MR back to the author:
- Reviewers field: Keep the reviewer in the Reviewer field.
- Assignee field: Unassign the MR from the person who reviewed and assign the author.
Do this until the MR is ready. When you're done reviewing:
- Reviewers field: Keep the reviewer as Reviewer.
- Assignee field: Unassign the reviewer from the MR.
To add yourself as a reviewer
If you're not set as Reviewer but was somehow else requested you to review the MR:
- Reviewers field: Add yourself as a Reviewer.
- Assignee field: Assign yourself to the MR.
Once your pass is done (if you want to pass the MR back to the author):
- Reviewers field: Keep yourself as a Reviewer.
- Assignee field: Unassign yourself and assign the author.
Do this until the MR is ready. When you're done reviewing:
- Reviewers field: Keep yourself as a Reviewer.
- Assignee field: Unassign yourself from the MR.
If the reviewer can't review the MR
If the person was added to the Reviewer field but, for whatever reason, couldn't review the MR:
- Reviewers field: Remove them from the Reviewers field
- Reviewers field: If someone else reviewed in their place: add the new person as a Reviewer.
Who can address the issue
development guidelines Technical Writing
Other links/references
I'm trying to propose something that doesn't affect so much the workflow we're used to while taking advantage of the new feature.