feat: Clarify approved vs mergeable for gitlab

[lukemassa]

what

• Make approved mean "Approved by a user other than the author" for gitlab
• Revert fix: Change message for when request is not approved to be more accurate #3744, as the original wording is now accurate
• Update docs to clarify that "approved" means "Approved by a user other than the author" regardless of VCS, whereas "mergeable" incorporates VCS-specific approval conditions.

why

Per #2605, gitlab does not respect the approved the way users would expect, i.e. has someone clicked the "approved" button. This is how the other VCSs interpret approved.

The current implementation for gitlab is closer to "are approval rules passing". That's what I thought "approved" meant when I implemented #3744 (since I'm most familiar with gitlab).

However, looking closer, it turns out:

• The other VCSs treat this as "someone other than the author clicked approve"
• gitlab itself already implements "follows approval rules" (it asserts that mr.ApprovalsBeforeMerge <= 0)

Thus there was this inconsistency, plus it meant that approved was strictly weaker than mergeable for gitlab.

The theory here I think is that it is nice to have one knob for users to turn that means "a user needs to approve this" vs "a user needs to approve this if the project thinks a user needs to approve this".

I personally will only use mergeable in my setup, since I have tight control over all the repos, but without a change like this, there's no way to implement #2605 , and force workflows to have approvals from the "atlantis side".

tests

Before:

apply_requirements	Project has MR Approval Rule	Project does not have MR Approval Rule
[]	Does not require approval	Does not require approval
[approved]	Requires approval	Does not require approval
[mergeable]	Requires approval	Does not require approval
[approved, mergeable]	Requires approval	Does not require approval

After:

apply_requirements	Project has MR Approval Rule	Project does not have MR Approval Rule
[]	Does not require approval	Does not require approval
[approved]	Requires approval	Requires approval
[mergeable]	Requires approval	Does not require approval
[approved, mergeable]	Requires approval	Requires approval

As you can see from the above, the functional change here is, for projects that do not have an MR Approval Rule, setting approved requires approval.

references

reverts #3744
closes: #2605

[jamengual]

@GenPage

[lukemassa]

Discussed this at office hours today, and @GenPage asked me to add a comment about the impact of this change.

If merged, this would be a breaking change for gitlab users who use the approved command requirement and who have repos that do not have approval rules. Commands in those repos would begin saying "Pull request must be approved by at least one person other than the author before running ". If users don't want to require approvals, they can remove approved. If they want approval rules to be evaluated, they can add mergeable (which respects approval rules, and whose implementation has not been changed by this PR).

[jamengual]

@lukemassa, could you sign the commits and fix the conflicts to merge this? Thanks.

[github-actions]

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.

[lukemassa]

UPDATE: I rebased and signed off on the commit. A lot of time has passed here; I no longer use gitlab so I don't quite feel qualified to say whether this is a step forward or not for gitlab. If gitlab users still think this makes sense I'm fine having it merged.

[github-actions]

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.