"Blocking Comments" on Merge Requests
This idea was born out of a conversation in #761 as a way to improve the MR workflow.
The problem space was well described by @cautionbug in that conversation:
- Project requires 2 approvals, sets 3 approvers (more than required so people can take vacations or have sick days, y'know...)
- MR comes in, Approver 1 OK's it.
- Approver 2 sees a problem, does not approve.
- Approver 3 doesn't see the problem, approves, and now can merge with the problem left unfixed.
- Or, developer end-runs around Approver 2 to get 3's approval. Hopefully we don't have that kind of person on the team, but...
The idea is to introduce a concept of a
Blocking Comment vs. a standard comment on Merge Requests. In SmartBear's Code Collaborator, this idea shows up as the difference between " add a comment" and "add a defect"; the latter of which blocks advancement of the review workflow. In GitLab this would probably simply show up as a checkbox option on the existing form.
- Only relevant/available when "MR Approvals" is enabled.
- Any participant can add a Blocking Comment.
- Adding Blocking Comment removes any previous approvals (similar to pushing of new code).
- A Blocking Comment can be
resolvedby any participant who can Approve the MR.
- Presence of a
unresolvedBlocking Comment prevents reviewers from approving the MR.
This feature also has some additional value-add potential in terms of future enhancements:
- Special notification options for
Blocking Commentsvs regular comments.
- A way to easily jump to, filter, or otherwise focus on blocking comments.
- A summary of how many resolved/unresolved blocking comments a MR has.
- A historical view of how often blocking comments show up in reviews to track code-quality trends.