[go: up one dir, main page]

Skip to content

Approve and unapprove a merge request in all merge request states and approval states

Individual issues to be worked on one-by-one to meet the designs and functionality in this issue

  • #4132 (closed) Approve and remove approval in all merge request states
  • #4133 (closed) Icons for approvals remaining and approvals met
  • #4134 (closed) Approve additionally
  • Issue to handle implementing Merging is blocked until sufficiently approved.
  • Improvements to displaying avatars and messaging. Different scenarios as shown below. To be separated into multiple issues.

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: avatar2

Requires 1 more approval

cannot approve or remove approval: MR-merged-environment-performance-1_Copy_79

can approve: MR-merged-environment-performance-1_Copy_71

can remove approval: MR-merged-environment-performance-1_Copy_86

Requires 5 more approvals ("approval" becomes "approvals")

cannot approve or remove approval: MR-merged-environment-performance-1_Copy_81

can approve: MR-merged-environment-performance-1_Copy_65

can remove approval: MR-merged-environment-performance-1_Copy_94

Requires 5 more approvals with 2 specified eligible approvers

cannot approve or remove approval: MR-merged-environment-performance-1_Copy_83

can approve: MR-merged-environment-performance-1_Copy_67

can remove approval: MR-merged-environment-performance-1_Copy_69

Requires 4 more approvals with 4 specified eligible approvers

cannot approve or remove approval: enough_approvers_copy

can approve (you are a eligible approver, last in the list): enough_approvers

can remove approval: enough_approvers_copy_2

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: enough_approvers_with_group_copy_5

can approve (you are a eligible approver, last in the list): enough_approvers_with_group_copy_8

can remove approval: enough_approvers_with_group

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: enough_approvers_with_group_copy_12

can approve: enough_approvers_with_group_copy_11

can remove approval: enough_approvers_with_group_copy_9

Received 2 approvals when 2 are required

cannot approve or remove approval: enough_approvers_with_group_copy_10

can approve additionally: enough_approvers_with_group_copy_2

can remove approval: enough_approvers_with_group_copy

Received 3 approvals when 2 are required

cannot approve or remove approval: enough_approvers_with_group_copy_6

can approve additionally: enough_approvers_with_group_copy_4

can remove approval: enough_approvers_with_group_copy_3

If their are no approvers yet, the second line is not shown:

MR-merged-environment-performance-1_Copy_98

If the MR is blocked by approvals, it should show like:

image

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 Coung Ngo