Count MR by approver as approval
Description
Required approvals combined with reset-approvals-on-push are 'broken' in the following scenario:
- User A and B are approvers for a project
- approvals-required is set to 1. Usually, this makes sure 4 eyes have looked at the MR (merge request), because you are not allowed to aprove your own MR.
- reset-approvals-on-push is turned on
- User A creates a MR to master, with commits [A1, A2].
- User B pushes any number of commits to the merge request branch, e.g. [B1, B2], so we have [master, A1, A2, B1, B2]
- User B approves
User B has now been able to approve his/her own code.
Proposal
We currently act from the idea that you should not be able to approve your own code - this manifests in not being able to approve your own MR (in case you have approval rights). We should not have this idea, because that inherently leads to complexities as described above. Instead, we should switch to the mathematically simpler idea of always being in approval of your own code or whatever you put in a MR.
I believe that the original MR should be counted as an automatic approval IFF the requester is a designated approver. That way, his/her approval can be retracted on push. Any organisation that wants to implement a four eyes principle would then set approvals-required to 2.
Note that while the original MR is an automatic approval by the user performing it for a commit history containing commits of any number of authors (probably by user A, but maybe also by user B and/or others). When user B pushes to the MR this is not an automatic approval for the MR at that point. Instead, person B is expected to review the entire MR, including commits of user A, C etc, and approve explicitly.
As a complimentary feature, if someone (let's say user A) adds WIP (work in progress) to the MR, his/her approval should be revoked. That way, if someone else (user B) removes WIP but user A does not agree, user B cannot merge by him-/herself.