Skip to content

"Blocking Comments" on Merge Requests

This idea was born out of a conversation in #761 (closed) 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.

General behaviors:

  • 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 resolved by any participant who can Approve the MR.
  • Presence of a unresolved Blocking 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 Comments vs 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.