Skip to content

Automatic merge and successful pipelines

Background

As part of #443310 (closed) it was identified that the behavior of Merge when pipeline succeeds is independent of any kind of merge check like require a successful pipeline for merge.

This creates a gap in how we now need to roll-out and communicate the automatic merge functionality. User expectation seems to be that pipeline success is "automatic" as part of enabling any kind of merge behavior. This leave us with a few questions we need to answer:

  1. How do we want to handle the loss of "MWPS" in the new paradigm of merging only around configured checks?
  2. How should we communicate this to users (in application, blogs, etc...)

Options

Option 1

We could consider officially (as opposed to indirectly) deprecating and removing the "Merge when pipeline succeeds" behavior. As part of this, users who would want to retain this behavior in the "Auto-merge" behavior would need to make a decision to enable the setting for requiring a successful pipeline. As part of this we'd also want to look at:

  • Adjusting the wording on the widget to make it clear that pipelines aren't evaluated by default
  • Create a breaking change notice and announce it in the release post

Option 2

We could still consider officially deprecating and removing the "Merge when pipeline succeeds" behavior, however instead of users needing to opt-in to the behavior for pipelines we'd perform a migration to enable the setting for users. This has the benefit of making sure that most users continue to function as they did before, but does remove some options to merge immediately or bypass things in case of CI failure. We'd still want to:

  • Adjust the wording on the widget to make it more clear what's happening
  • Create a breaking change notice and announce it in the release post

Option 3: "Keep the WPS"

We could modify the AutoMerge::MergeWhenChecksPassService to use the existing merge_requests.merge_when_pipeline_succeeds_column. The workflow would be:

  1. (exists) This should use whatever existing logic we use to detect running CI and offer MWPS.
  2. (exists) If CI is running and we offer "auto-merge", we should set merge_request.merge_when_pipelines_suceeds to true.
  3. (exists) Currently, AutoMerge::MergeWhenChecksPassService optionally checks CI status if the Project Pipeline must succeed setting is enabled.
  4. (to build) We should also check CI status in the exact same way if merge_request.merge_when_pipeline_succeeds is set to true. If CI is running and MWPS is true, we treat the MR as unmergable in the exact same way we would if the Project setting was enabled. These two columns will have the same effect, but Pipelines must succeed is scoped to every MergeRequest in the Project whereas merge_request.merge_when_pipeline_succeeds is scoped only to the single MergeRequest.

Related thoughts:

  • If a MergeRequest has merge_when_pipelines_succeed set to true, we should probably populate that into the widget list as a "check", with some distinct language, e.g. "This MR has been set to require passing CI before merging". Something to differentiate from the Project-wide policy.
  • We'd probably need to keep allowing Merge Immediately for projects that don't have Pipeline must succeed enabled. I don't think this will require changes, it will continue to operate normally based on the Project Pipeline must succeed setting.
  • The current MWPS functionality would still work, based on the same column. Since we're not changing the semantic value of the column by "repurposing" it for use in AutoMerge service, turning the flag on or off, rolling back the feature, etc. won't break merge logic on in-flight MRs.
  • This workflow would more clearly (IMO) articulate the purpose of MWPS, which is that "a maintainer says this is okay to merge, contingent on the result of CI". Right now, MWPS puts that decision in the hands of individual maintainers, and the Project setting allows for owners/admins to set the same policy universally. This concept was just sort of implied before, but it exists and is clearly relied on by project maintainers. If we want to remove it, we still can later, but we'll be able to talk about it in a more well-defined way.

Option 4: Solving the immediate merge gap

We could introduce a new setting (ugh, I already hate it) that's called Allow immediate merge which is nested below the currently existing setting of require a successful pipeline so it can only be toggled if this is enabled.

If you choose to allow immediate merge then if a running pipeline is the only remaining check the dropdown is available to merge immediately and bypass the pipeline. If there are other checks outstanding then the merge immediately option isn't available.

We would then couple this with Option 2 so that we transition everyone to the existing setting which gives them the behavior that they've all got implicitly today.

Option 5

????

Edited by drew stachon