Optimistic batching for batch merging #252
Conversation
Fix for issue smarkets#164. Previously we added trailers to each MR and merged the constituent MRs of a batch MR to the target branch. Now we will now optimistically add the trailers to each MR and rebase on the batch branch. And when the batch branch passes CI we merge it which will automatically merge each constituent branch.
| @@ -225,36 +215,34 @@ def execute(self): | |||
| working_merge_requests.append(merge_request) | |||
| if len(working_merge_requests) <= 1: | |||
| raise CannotBatch('not enough ready merge requests') | |||
| if self._project.only_allow_merge_if_pipeline_succeeds: | |||
There was a problem hiding this comment.
We seem to have lost this check. Whilst batching is a bit weird if you don't have pipelines enabled, we'd still want to support simply merging the batch branch in without waiting for any pipelines (which won't exist now) to finish.
marge/batch_job.py
Outdated
| @@ -207,6 +194,9 @@ def execute(self): | |||
| local=True, | |||
| ) | |||
|
|
|||
| # Ensure that individual branch commit SHA matches matches that of its equivalent in batch MR | |||
| self.push_force_to_mr(merge_request, True, source_repo_url, skip_ci=True) | |||
There was a problem hiding this comment.
I'm not sure we want to push here - the source branch for the MR will now contain commits of other MRs that have previously been added to the batch branch - we don't want to mess with the original MRs here, especially as the batch can fail.
| for merge_request in working_merge_requests: | ||
| try: | ||
| # FIXME: this should probably be part of the merge request | ||
| _, source_repo_url, merge_request_remote = self.fetch_source_project(merge_request) | ||
| self.ensure_mr_not_changed(merge_request) |
There was a problem hiding this comment.
Why has this check been lost?
marge/batch_job.py
Outdated
| @@ -198,6 +182,9 @@ def execute(self): | |||
| merge_request.source_branch, | |||
| '%s/%s' % (merge_request_remote, merge_request.source_branch), | |||
| ) | |||
| # Apply the trailers before running the batch MR | |||
| self.add_trailers(merge_request) | |||
| self.push_force_to_mr(merge_request, True, source_repo_url, skip_ci=True) | |||
There was a problem hiding this comment.
It would be nicer to specify keyword args for the boolean, as makes it easier to know what this is doing (i.e. branch_was_modified=True)
marge/git.py
Outdated
| skip_2 = 'ci.skip' | ||
| else: | ||
| skip_1, skip_2 = '', '' | ||
| self.git('push', force_flag, skip_1, skip_2, source, '%s:%s' % (branch, branch)) |
There was a problem hiding this comment.
I think you can just have:
skip_flag = ['-o', 'ci.skip'] if skip_ci else []
and then:
self.git('push', force_flag, *skip_flag, source, ...)
This reverts commit 71f9754.
This commit is still missing the logic to ensure that the individual MR hashes and those in the batch MR match
And added docs on the new faeture
Fix for issue #164.
Previously we added trailers to each MR and
merged the constituent MRs of a batch MR to
the target branch.
Now we will now optimistically add the trailers
to each MR and rebase on the batch branch. And
when the batch branch passes CI we merge it
which will automatically merge each constituent
branch.