Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

Rebase and merge pull request option should add a merge commit #1143

Open
johnnyoshika opened this issue Dec 15, 2017 · 56 comments
Open

Rebase and merge pull request option should add a merge commit #1143

johnnyoshika opened this issue Dec 15, 2017 · 56 comments

Comments

@johnnyoshika
Copy link

Here are the options for merging pull requests:
image

Selecting the first one (Create a merge commit) will create a new merge commit so that you can easily tell that a merge was done through a pull request when you look at git log. The Rebase and merge option, however, won't create a merge commit. I think it should do so. So in other words, Rebase and merge should first rebase, and then do the same thing as the Create a merge commit option.

@johnnyoshika
Copy link
Author

Note: I sent this enhancement request to [email protected]

@johnnyoshika
Copy link
Author

GitHub's response:

Hey Johnny,Thanks for getting in touch! We've passed your request to the team internally for consideration, though we can't make any promises of specific changes. Let us know if you have any other ideas you'd like to share with us.

All the best,GitHub Staff

@yogasantosa
Copy link

I was evaluating and was excited when they offer "rebase and merge" - but then I checked the documentation and saw that rebased merge is fast-forwarded (https://help.github.com/articles/about-pull-request-merges/):

Pull requests with rebased commits are merged using the fast-forward option.

Would be nice if there's an option to have the merged --no-ff'd.

And it seems this open ticket of yours here confirms that feature isn't there yet. I wish I could vote for this feature.

@johnnyoshika
Copy link
Author

johnnyoshika commented Jan 17, 2018

@yogasantosa I'm anxiously waiting for this feature. --no-ff is exactly what we need.

@yogasantosa
Copy link

@johnnyoshika - the other thing i'm not sure either is: --preserve-merges: I don't think they support it either.

@johnnyoshika
Copy link
Author

@yogasantosa --preserve-merges would be nice, but I the feature I miss the most is --no-ff.

@Wildeast
Copy link

Just for understanding: What does fast forward mean for a "Rebase" ?
I mean, rebase creates new commits, so there is no fast forward (in the meaning of just updating branch pointer and not creating new commits).

@johnnyoshika
Copy link
Author

@Wildeast GitHub offers the option to Rebase and merge when merging Pull Requests (see screenshot in the first comment). Whey they do that, they rebase and then merge using fast-forward. What I'd like is an option to rebase and then merge without fast-forward.

Note: Rebase without fast-forward is the default merge option (GitHub calls it Create a merge commit). I'd like to have that option when rebasing as well.

@pvdlg
Copy link

pvdlg commented Jul 5, 2018

I don't think Rebase and merge do a rebase of the head branch (the topic/feature branch) from master and then a git merge. What it does is that it rebase the topic/feature branch on top of master. There is no merge commit, because it's the point of the Rebase and merge feature.

This is what is mentioned in the chapter [Rebase and merge your pull request commits]:

When you select the Rebase and merge option on a pull request on GitHub, all commits from the topic branch (or head branch) are added onto the base branch individually without a merge commit.

To clarify the Merge pull request feature does:

git checkout master
git merge --no-ff feature
git push origin master

The Rebase and merge feature does:

git checkout master
git rebase feature
git push origin master

What you seems to think it's doing is:

git checkout feature
git rebase origin/master
git checkout master
git merge --no-ff feature
git push origin master

Basically this is master that is rebased (with the commits of the feature branch). What you seems to suggest is that it's the feature branch that is rebased then merged.

This is hinted at in [Rebase and merge your pull request commits]:

The rebase and merge behavior on GitHub deviates slightly from git rebase.

So GitHub Rebase and merge use git rebase and not git merge. The difference between Rebase and merge and git rebase is that GitHub always rewrite the commits sha (to change the committer info) while git rebase does it only if necessary.

Moreover it is later mentioned:

You aren't able to automatically rebase and merge on GitHub when:

  • The pull request has merge conflicts.
  • Rebasing the commits from the base branch into the head branch runs into conflicts.
  • Rebasing the commits is considered "unsafe," such as when a rebase is possible without merge conflicts but would produce a different result than a merge would.

Those constraints a due to the fact that Rebase and merge indeed uses git rebase to rebase master with the commit of the feature branch. If it was doing a rebase of the feature branch from master and then a git merge those constraint would exists.

The documentation is quite misleading because in the chapter Rebase and merge your pull request commits it's also mentioned:

Pull requests with rebased commits are merged using the fast-forward option

This sentence seems wrong (probably of copy/paste of Pull requests with squashed commits are merged using the fast-forward option ?) because there is no notion of fast-forwarding if you rebase the topic branch on top of master.

@johnnyoshika
Copy link
Author

Thanks @pvdlg for the explanation. It's a lot more clear now. I was hoping that we can get a new option that did this:

git checkout feature
git rebase origin/master
git checkout master
git merge --no-ff feature
git push origin master

@pvdlg
Copy link

pvdlg commented Jul 6, 2018

I'm not sure it's a good idea, because after git checkout feature and git rebase origin/master you want the CI to run you your tests and make sure everything is ok before you run git merge --no-ff feature and git push origin master.

What you are asking is already possible now by doing as follow:

  • On the PR click on Update branch
  • Wait for the CI to pass to make sure it didn't break something
  • On the PR click on Create a merge commit as you want to merge with a merge commit

@pronebird
Copy link

pronebird commented Jul 15, 2018

@pvdlg I just played around on a simple repo and it seems that rebasing master on top of feature branch simply adds commits from feature branch to master.

At least on my mac it happens in the historical order. So it's likely that the one has to fix some conflicts during rebase and that it's a good idea to wait for the CI anyway, except it's impossible to achieve since the commits are already in master at this point.

However, I guess Github shields users from such scenarios and requires manual resolution/merge etc..

It's confusing that this specific option is called "Rebase and merge" because there is no merge. That's why everyone thinks that it's a rebase on top of master with the subsequent merge to master.

@tmedioni
Copy link

In don't understand the latter part of this thread. Rebasing master on top of feature branch would change master's commit shas if the branches diverged both sides ? It does not seem to be the case though (and hopefully so)

@chaoliu2002
Copy link

Is it possible to make 'Rebase and merge' the default option?

@RobvH
Copy link

RobvH commented Oct 1, 2018

I would be satisfied with just a strict check requiring a PR's base to be at the target branch's HEAD before merging. An Update button would be useful if it did a rebase. Half the reason we rebase incoming branches is to remove any intermediate merge commits. We are against squashing away useful history, but for squashing away WIP commits, etc. This maintains a nice clean, serial commit flow with good history but without garbage commits and has supported our trunk based flow very well. Some of this is aesthetics -- but I think there is value in the tree being very simple and easy to visually comprehend.

@RobvH
Copy link

RobvH commented Oct 1, 2018

the 'Rebase and Merge' option isn't that -- it ought to be renamed, 'rebase and ff'

@edwardchapin
Copy link

I ended up here because I am in the process of experimenting with the Large Synoptic Survey Telescope workflow for another astronomy project, and it seems to be identical to what is being discussed here. An example of one of their repos in the wild is here: https://github.com/lsst-dm/fgcmcal.git It's nice because you get the clean/linear history of a purely rebased workflow, but the non-ff merge commits encode the name of the branch (which is useful for project management, and saves you having to add it to the individual commit messages). Note that the merging section from that LSST guide clearly describes the problem with the GitHub interface (and why they end up doing merges from the command-line instead). So, another vote from me to add the ability to enforce non-ff merges.

@bekriebel
Copy link

@pvdlg commented:

What you are asking is already possible now by doing as follow:

  • On the PR click on Update branch
  • Wait for the CI to pass to make sure it didn't break something
  • On the PR click on Create a merge commit as you want to merge with a merge commit

This creates a merge commit from the target branch into the PR source branch. This is nearly the opposite of what is being requested. If the Update branch button had an option to perform a rebase, that would be fantastic. As it is, there's no way to do a rebase from the UI without also fast-forwarding it during the approval of the PR.

@joaoe
Copy link

joaoe commented Nov 6, 2018

About "rebase and merge" creating a merge commit is a -1000 from me.
Unless you have a really good reason to (e.g. merging two monster branches) you should keep your branch history clean and easy to read, which means a single chain of commits, and not the spaghettification typically of people committing and merging often, which makes the branch history useless as a form of documentation.
So, a rebase and merge which produces a clean merge commit without the need for conflict resolution is good to have, but the best option is to just rebase your changes on top of master..

@itaranto
Copy link

It would be nice to have a fourth option equivalent to git rebase + git merge --no-ff

So the three buttons (excluding squash and merge) would be :
Merge pull request -> git merge --no-ff
Rebase and Merge 1 -> git rebase + git merge --no-ff
Rebase and Merge 2 -> git rebase + git merge

@peter-leonov
Copy link

I is being discussed for half of the lifespan of GitHub here and there. Is there a bad architecture violation / performance bottleneck we all overlook here which blocks GitHub from just adding an option?

@stefanbauer
Copy link

+1 for Rebase, Squash and Merge with --no-ff

@BrentMifsud
Copy link

Id like to see another rebase option. One that doesn't create a new set of SHAs.

Every time I rebase merge, I now have to either delete my feature branch, or manually reset + force push it to get to the same point as the base branch.

@waldyrious
Copy link

Id like to see another rebase option. One that doesn't create a new set of SHAs.

Isn't that's the regular merge option? If you rebase, by definition the commit SHAs of the branch will change (because their ancestry chain, which is part of what determines their hash, changes when you change the base commit of a branch). Unless I misunderstood what you mean?

@BrentMifsud
Copy link

What I mean is, normally when you rebase. the base branch is fast forwarded to the feature branch. Same commit SHA.

on GitHub, when you choose rebase and merge, it essentially copies those commits and makes new SHAs.

Here is an example:

This is the before: feature/v3.16.5 is the branch that I want to rebase and merge onto develop.
Screen Shot 2020-06-26 at 11 54 27 AM

This is what happens when you tap rebase and merge on GitHub:
Screen Shot 2020-06-26 at 11 54 34 AM

It essentially copied the commit from the pull request, and gave it a new SHA.

When I choose to rebase, my expectation is that the base branch would now be on the same commit SHA as the pull request branch. At least, that's the behaviour if you do a git rebase on the command line or from a GUI tool:

Screen Shot 2020-06-26 at 11 54 49 AM

@waldyrious
Copy link

Gotcha — you're saying that if the PR branch is already up-to-date, a "rebase and merge" should only do a fast-forward merge, rather than force a rebase (which implies making copies of the commits) even though it isn't needed.

I agree that an option to do fast-forward merges would be desirable, although I'd prefer to see that as a separate option called explicitly "fast-forward merge", to avoid exacerbating the merge vs. rebase confusion that's already quite common.

@pronebird
Copy link

pronebird commented Jun 29, 2020

Please add "Rebase and merge with --no-ff". I can rebase manually, no problem but PRs then are not detected as merged and that hurts the feelings of contributors.

@kszorin
Copy link

kszorin commented Aug 9, 2020

Github, please add this necessary feature!

@peter-leonov
Copy link

Or simply hire me for 0$ until I manage to make this feature come to life 😅

@chucklu
Copy link

chucklu commented Aug 14, 2020

Azure devops also has this feature, when will you implement this feature?
https://devblogs.microsoft.com/devops/pull-requests-with-rebase/
Semi-linear merge

This strategy is the most exotic – it’s a mix of rebase and a merge. First, the commits in the pull request are rebased on top of the master branch. Then those rebased pull requests are merged into master branch. It emulates running git rebase master on the pull request branch, followed by git merge pr --no-ff on the master branch.

Some people think of this as the best of both worlds: individual commits are retained, so that you can see how the work evolved, but instead of just being rebased, a “merge bubble” is shown so that you can immediately see the work in each individual pull request.

@mbitsnbites
Copy link

I honestly think that #1017 has a much cleaner description and scope. It requests that a new, fourth alternative is added ("Create merge commit with semi-linear history").

@JohnyWS
Copy link

JohnyWS commented Dec 13, 2020

I agree, this has ended up being exactly the feature request that #1017 is - and as pointed out previously, it in better detail in #1017, is that is already implemented in other git servers out there, and even Azure DevOps also owned by Microsoft. ;) I'd really like to see this added, even with a "lock this repo to ONLY use this option". :) Merges should then always be the PR description, and you're good to go on delving in all you want on how files have changed.

Should these two issues be merged? If so, I vote for #1017 to be the main one going forward.

@farislr
Copy link

farislr commented Apr 28, 2021

Is this requested feature similar to git rebase -m ?

@chucklu
Copy link

chucklu commented Apr 28, 2021

@farislr No.
rebase -m

Use merging strategies to rebase. When the recursive (default) merge strategy is used, this allows rebase to be aware of renames on the upstream side. This is the default.

It has nothing to do with semi-linear merge which implement by rebase first, then no-fast-forward merge.

@AraHaan
Copy link

AraHaan commented Apr 28, 2021

The thing about merge commits themselves is that they are empty and useless to me, for starters I like my tree to be linear and rebase and merge does that.

Now I would love it if there was a new button however and that is rebase + squash, I think that is also a thing I would really love as well.

For starters I think having rebase and merge doing a merge commit is a good thing, but only for projects who have policy to make them, some people prefer rebase and merge without merge commits and so they say "create merge commit, you fired!".

@michalmela-tomtom
Copy link

The thing about merge commits themselves is that they are empty and useless to me, for starters I like my tree to be linear and rebase and merge does that.

@AraHaan If you rebase first, the "non-linearity" introduced by a merge commit is really negligible. It's the non-rebased merges that make the commit tree look awful.

And the merge commit is not completely useless, it (put simply) groups the commits that belonged to a single PR and it may contain a link to that PR. Then a person analyzing the history of a git repo, looking for some information, may find something useful in the comments that were made when reviewing the PR.

All of that in addition to some formal requirements (e.g. gov projects that need to be able to prove a formal review process was made for each and every change) that some repositories may need to fulfill, as you have very aptly observed.

@AraHaan
Copy link

AraHaan commented May 13, 2021

On my projects I set mine up on rebase and merge to include (#<pr number>) in the last bit of the commit message for the title of the commit successfully without making merge commits. As such many workflows including cpython btw rely on rebase merges and cherry-picking.

@chucklu
Copy link

chucklu commented May 13, 2021

@AraHaan rebase + squash does not exist. Have you ever used the squash feature?
You might refer to What is the difference between merge --squash and rebase?

From my experience, I never use the squash feature. It's weird to drop the history commits.

@AraHaan
Copy link

AraHaan commented May 13, 2021

@chucklu actually it can be done, on mergify I set my update strategy from merge commit to rebase and then have it squash merge. However I wish to have github handle it instead so that way I would not have to wait for the PR's own status checks to pass first (only when from github native dependabot) (since the thing will be merged anyway) and so then the status check reruns could be done on the branch it gets merged into (and if it fails then it could be reverted then and redone correctly to ensure the default branch remains buildable at least).

@chucklu
Copy link

chucklu commented May 13, 2021

@AraHaan semi-linear is rebase + git merge pr --no-ff, and squash is just git merge pr --squash.
Squash is just a merge policy, if you want to rebase + squash, then the first rebase is useless.

Let me show you an example
C1<--C2<--C3 branch1
C1<--C4<--C5 branch2
if you want to squash branch2 into branch1, just use squash command is enough,
you will get C1<--C2<--C3<--C6 (C6 is the squash result of C4 and C5)

if you use "rebase+squash", it means rebase first then squash
you will get C1<--C2<--C3<--C4'<--C5' (rebase operation)
then sqush C1<--C2<--C3<--C7(C7 is the squash result of C4’ and C5')
Why you would like to do the useless rebase before suqash?

@AraHaan
Copy link

AraHaan commented May 13, 2021

Sometimes when 2 prs are merged at almost the same millisecond the second one might actually try to merge in a conflicted change while a rebase and then the squash as a button could resolve the conflict by having it know of the changes previously merged just prior without needing to wait for the checks to pass again (and as such uses less github actions minutes then).

While chances of conflicts could possibly be low it can possibly happen even when we do not realize it at that moment until the automated merges leaves that branch in a non-buildable state with some possible merge conflict annotations in some files. When I do things like this locally I usually avoid merge conflicts by rebasing pr's on top of the tip of the target branch first to try to reduce the changes of conflicting the pr's merge itself.

@chucklu
Copy link

chucklu commented May 13, 2021

@AraHaan Have no experience on auto merge(who review the code?), how could it resolve the conflict? From my experience the conflict always need to resolve manually.

Sometimes when 2 prs are merged at almost the same millisecond

The 2 prs should be arranged in a queue, and then it can be done one by one.

@waldobronchart
Copy link

For anyone watching this thread, I wrote a script to do this because I was tired of waiting on Github.

Get the script here: https://pypi.org/project/git-pr-linear-merge/

It does what the original post describes: rebase, then merge without fast-forward with a simple terminal command:
git pr merge XXX

Documentation can be found on the project page.

@nrjohnstone
Copy link

I can't believe this is not a feature, we have been using bitbucket for years and the rebase + merge commit is important for all the reasons mentioned here (auditability, easy to revert a single merge commit rather than 50 individual commits etc..) Make it at least an option so all the people who work on toy projects that don't need a merge commit don't feel offended, but at least for people that work in large teams with lots of contributors, allow us to have rebase with a merge commit !

@chucklu
Copy link

chucklu commented May 21, 2021

@nrjohnstone Actually this is not an official issue tracker for github, you can send feedback to github via https://support.github.com/contact/feedback

@Kurt-von-Laven
Copy link

In don't understand the latter part of this thread. Rebasing master on top of feature branch would change master's commit shas if the branches diverged both sides ? It does not seem to be the case though (and hopefully so)

@tmedioni, this is why GitHub won't allow you to merge in any fashion whatsoever with their shiny green buttons when there is any merge conflict whatsoever between the head and base branches.

Is it possible to make 'Rebase and merge' the default option?

@chaoliu2002, @JohnyWS sort of, but unfortunately only by making it the only option using the settings described here.

@hanscho
Copy link

hanscho commented Aug 20, 2021

+1
Please prioritize this as described by #1143 (comment)

Rebase on master and then merge without fast forward yields a very nice and readable history where you can easily see all the pull request merges having been done.

To those that think that the history should be linear: I envy you for already having your option and I don't want it taken away. So there should be a new option, or a configuration for deciding whether "Rebase and merge" should use --no-ff. Preferably a new option.

To those thank think rebasing and creating a merge commit could break tests and whatnot. Well, of course, but what do you think the existing "Rebase and merge (with fast-forward)" could do?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests