Skip to content

Check fast-forward more precisely

username-removed-423915 requested to merge check-ff-more-precisely into master

The original issue is that (#260 (closed)), some people want to be able to merge a fast-forward MR regardless it's rebased or not. The previously check for this: target_sha == source_sha_parent, assumes that the questioning MR has a linear history (i.e. it's rebased), therefore we could do a fast-forward merge. However, we could also do fast-forward merge if the MR has already merged target branch (i.e. it's merged).

This MR would allow them to merge any fast-forward MR regardless rebased or not.

But actually this breaks some assumption. In the option of:

Merge commit with semi-linear history

A merge commit is created for every merge, but merging is only allowed if the branch has been rebased. This way you get a history that reads linearly (as with fast-forward merges), with the addition of merge commits. When the branch has not been rebased, the user is given the option to do so.

It clearly says: merging is only allowed if the branch has been rebased This MR would break this assumption. The same applies to:

Fast-forward merge

No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch has been rebased. When the branch has not been rebased, the user is given the option to do so.

This means that rebase and FF are two separated concept and we're mixing them here. We should probably update the wordings and perhaps provide another option if we want to keep all the workflows.

My own thought is actually, allowing only FF merge might make little sense. Allowing only FF and rebased (linear history) would make more sense. That means I think the original behaviour actually makes more sense, just that the wordings are wrong. i.e. They could be FF merged, but that's not the point, we still want you to rebase this branch!

On the other hand, I am not against adding a new option for this behaviour either. People have different workflows, and I think it makes sense to allow any workflows which Git provided.

Either way, I think some wordings should be changed.

By the way, could we write a test for this?

Fixes #260 (closed)

/cc @DouweM

Merge request reports