Skip to content

WIP: Require all commits in a MR to have a green pipeline before merging

As described in https://gitlab.com/gitlab-org/gitlab-ce/issues/13187, we want an option to block a merge request unless all commits have a successful pipeline. Currently, there's an option to require that the last commit is green, but nothing that requires this of all the MR commits. This is important to maintain build hygiene in workflows that rebase or merge without squashing.

There are two parts to this work:

  1. Definition of mergeable
  2. When pipelines are created

The new definition of mergeable has an allowance for the option to make a MR mergeable only if every pipeline succeeds. My proof of concept branch replaces the old logic with this, which should be easy to guard behind a project settings flag in a real implementation.

The second part is to modify the hook which creates pipelines. The current behavior is to create a pipeline per push, but we want to create a pipeline per commit. So that when a push contains multiple commits, all of the new commits get a pipeline. I also removed the code to cancel pending pipelines, with this change we want those pipelines to complete, since we care about every pipeline succeeding.

I did my best to reflect this approach in the code. I expect that a rigorous test will reveal areas that I missed (e.g. other scenarios that need to determine if a MR is mergeable must use merge_request.rb or else they will not get my updates).

Future work:

  • Testing. I would have liked to develop this test first, so I looked through the docs and tests to find testing examples that I could adapt. I think this is where I could have used a lot of help. I ended up abandoning a TDD approach and just tested manually. This approach is of course much slower and less consistent. If I had more time, I'd take this as the next step.
  • UX. There are various places where the copy assumes a single pipeline per MR (e.g. "The pipeline for this merge request failed") that need to be updated or reviewed to see if it still makes sense.

Merge request reports