feat: Adding support for setting required number of reviewers on a S…

[dkichler]

…calaSteward managed GitLab MR (#2519)

Closes #2519

[codecov[bot]]

Codecov Report

Base: 80.62% // Head: 80.76% // Increases project coverage by +0.14% :tada:

Coverage data is based on head (2bfde62) compared to base (bdb8c34). Patch coverage: 97.05% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2528      +/-   ##
==========================================
+ Coverage   80.62%   80.76%   +0.14%     
==========================================
  Files         147      147              
  Lines        2740     2761      +21     
  Branches      175      197      +22     
==========================================
+ Hits         2209     2230      +21     
  Misses        531      531              

Impacted Files	Coverage Δ
...ala/org/scalasteward/core/application/Config.scala	100.00% <ø> (ø)
...rg/scalasteward/core/vcs/gitlab/GitLabApiAlg.scala	73.14% <96.29%> (+4.33%)	:arrow_up:
.../scala/org/scalasteward/core/application/Cli.scala	100.00% <100.00%> (ø)
...n/scala/org/scalasteward/core/vcs/gitlab/Url.scala	61.53% <100.00%> (+6.99%)	:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[exoego]

@dkichler Sorry for the late response. Could you fix the conflict? Code looks great to me.

[dkichler]

Conflict addressed.

[dkichler]

@exoego I think this one is finally ready to go in.

[exoego]

Thanks!

[henricook]

@exoego Thanks for this. OOI why does it only take effect when merge if pipeline succeeds is in play?

This would be helpful for me because we like to allow only one approver for dependency bumps instead of two (for all other code changes)

[exoego]

Sorry, I don't use GitLab so I can not tell.

[dkichler]

@exoego Thanks for this. OOI why does it only take effect when merge if pipeline succeeds is in play?

This would be helpful for me because we like to allow only one approver for dependency bumps instead of two (for all other code changes)

Hi Henri. So the mergeWhenPipelineSucceeds flag perhaps has a slightly misleading name. If you inspect the code, the flag is used to gate whether the MR in question is automatically marked to be merged by the update algorithm. When set, the MR will also be adjusted with the configured number of required reviewers, as per this MR, prior to attempting the merge. The flag was named as such b/c in the attempt to merge, the query param for merging if pipeline succeeds is hardcoded.

I didn't want to change the semantics of the existing mergeWhenPipelineSucceeds flag, to avoid changing behaviour of any existing implementations, but I think it would be more appropriately named something like mergeUponAllCriteriaMet. Certainly there is some room to make the Gitlab algo implementation a bit cleaner/more explicit/configurable, but even as is I would expect it should account for most needs/use-cases. Given my explanation, does it still cover your use-case?

[henricook]

Thanks @dkichler. I think what you're saying here (to read it back) is that mergeWhenPipelineSucceeds can be set to true, but won't error if the MR is not in a ready to merge state (e.g. it still has outstanding approvals), so enabling it in my situation won't hurt anything - because if approvers are set to 1, 1 approver will be outstanding and no auto merge will be attempted.

Assuming I've understood that right then it sounds good for my use case, thanks!

[dkichler]

That's correct - setting it to true will apply any configured required reviewer changes to the MR, and then attempt to merge it. The merge attempt will always be gated by the MR being in CanBeMerged status, which will depend in your case on whether anyone has reviewed it or not (and any other merge requirements you might have set in Gitlab).

[henricook]

Lovely, thanks!

[henricook]

Sadly this didn't work out for me, but I don't blame Steward. The /approvals API that this MR used is only available in Gitlab Premium and we're currently on the old 'Gitlab Starter' package (that's no longer sold, but we're still in subscription for).

The only way to change approvers is by POSTing to the MR and the approvals API 404s until we upgrade.

Example of updating approvers via MR POST:

[dkichler]

Aww shucks, indeed it appears this feature does depend on Gitlab Premium. I think at the very least the documentation for the flag ought to be updated to state as much. To be fair, the Gitlab API is a bit of a mess to work with in the first place, it looks like the endpoint this feature is based on has already been deprecated and will be removed in an upcoming release.

Unfortunately, the replacement API (create/update/delete MR level rules) appears to also only have support in Premium 😞.