Show which eligible approvers have commented on the merge request
Problem to solve
As a merge request reviewer, it is important to know if a peer has already reviewed or commented on a merge request so that duplicate efforts are avoided and time is used efficiently.
In order to show which users have provided feedback via comments, the merge request widget has been updated to show a "Commented by" column, which clearly indicates who has provided feedback thus far. This will allow both authors and reviewers to promptly identify which peers to engage.
Further details
As a prospective reviewer/approver, I want to know if a relevant peer has already commented on (reviewed) the merge request, so that I don't pile on or use my own time inefficiently. If a relevant peer has already commented on but has not approved the merge request, this is very important information.
Currently the only way I can answer these questions is to skim through the entire merge request and try to spot the people I know. This works fine in a small team, but as teams grow and there are hundreds/thousands of engineers this doesn't scale. It is also not inclusive of new team members who have not learned who should review what.
Currently merge requests have the concept of approvals and resolvable discussions – these seem like related concepts and can probably be reused and avoid formally requiring someone to submit a “review”.
Proposal
From the perspective of an eligible approver, a merge request can be:
-
waiting(default): I have not commented on or approved it -
commented: I have left one or more comments, but I have not approved it -
approved: I have approved it (may or may not have left comments)
We already show who has approved a merge request. Using the states above we should also show how has commented on the merge request.
- Show
Commented bycolumn on the MR widget and populate it with eligible approvers who have left comments.
Possible future steps
- Style commented by and approved by users differently (example, different colored borders)
- Add count of unresolved threads per approver
Documentation
What does success look like, and how can we measure that?
This is a quality of life improvement that in combination with https://gitlab.com/gitlab-org/gitlab-ce/issues/58721 should reduce the average amount of time reviewers spend per session looking at the merge request because they are able to find their information faster. The number of sessions should be relatively constant.
This should support continued growth and adoption of merge requests by existing customers.