Approve and unapprove a merge request in all merge request states and approval states
Resources
PM @victorwu | UX @dimitrieh
Background
- Currently, approvals are used to block a merge request from being merged.
- However, when a merge request is already merged, there are no strict requirements. However, in GitLab currently, you cannot continue approvals after a merge request has been merged.
- This makes the product unnecessarily complicated. We want to make GitLab as simple as possible. One way is to reduce the number of rules. The proposal here is to allow approving/unapproving even when a merge request is merged. The benefit is that we reduce the complexity in the product, reduce number of business rules.
- Another benefit is that reviewers can express their signals on a merge request, even after it has been merged. Reviewers can come back and continue to approve / unapprove. The UI design should make it super obvious that the signal doesn't impact the merge request (it is already merged). And so there should be very little confusion about it. But this allows reviewers to continue expressing their signal. For example, if there are 10 approvers, but 3 already approved (and 3 was the required number). The remaining 7 can come back after the fact and continue talking and express their signals in the merge request. We should not restrict them. It solves the problem of unnecessarily restricting users/reviewers in discussing a code change, even after it has been merged.
Docs blurb
You can now approve merge requests (or remove your approval), even though the required number has already been reached.
Description
- If the number of approvals is not met, it should block the merge request from being merge-able.
- Achieving the required approvals is a necessary condition for the merge request to be merge-able, but not sufficient.
- If somebody unapproves and falls below the threshold, the merge request is blocked again.
- Under all merge request states (open/closed/merged and reached/not reached enough approvals), a qualified user should be able to approve or unapprove the merge request.
- Even if you have reached the required number of approvals, you can continue to approve and unapprove.
- Under all merge request states (open/closed/merged and reached/not reached enough approvals), view the status of the approvals.
- View who are explicit qualified approvers, including the users / groups that have been selected in either the project settings or the overwritten merge request settings.
- View who has approved. View who has not approved. View how many need to be approved / how many left to be approved.
- Iterate on the existing UX design. There is feedback that the existing empty dotted circle is confusing.
Design
Design
Screenshot are from the perspective of:
Requires 1 more approval
cannot approve or remove approval:
Requires 5 more approvals ("approval" becomes "approvals")
cannot approve or remove approval:
Requires 5 more approvals with 2 specified eligible approvers
cannot approve or remove approval:
Requires 4 more approvals with 4 specified eligible approvers
cannot approve or remove approval:
can approve (you are a eligible approver, last in the list):
Requires 4 more approvals with 2 specified eligible approvers and 1 specified eligible approver group (with enough people in it to get all required approvals)
cannot approve or remove approval:
can approve (you are a eligible approver, last in the list):
Requires 8 more approvals with 2 specified eligible approvers and 1 specified eligible approver group (withuot enough people in it to get all required approvals)
cannot approve or remove approval:
Received 2 approvals when 2 are required
cannot approve or remove approval:
Received 3 approvals when 2 are required
cannot approve or remove approval:
If their are no approvers yet, the second line is not shown:
If the MR is blocked by approvals, it should show like:
Notes:
- If there are less eligible specified approvers (including those in the specified approver groups),
and others
is mentioned. - If you yourself can approve, your avatar always shows last in the eligible approver avatar list on the first line.
- If you yourself did approve, your avatar always shows last in the approver avatar list on the second line.
- If you aren't a named approver, or a member of an approver group, you can (currently) only approve if there are more approvals required than people who are named approvers or members of approver groups (this currently is already functioning this way). Please see the following doc: https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html
Edited by Dimitrie Hoekstra