9 ms·

Rebase and merge pull requests

181 points by dwaxe 10y ago

poorman 10y ago
Can someone explain the benefit of this? vs. "squash and merge"?
- crugej 10y ago
  If you want to keep the individual commits rather than squash the into a single commit. This is useful if the author of the PR actually wrote good commits that you don't want to make into a single commit. I'm also thinking of the case where a squash could turn a series of commits into a single commit with a massive amount of changes that is harder to grok when going back historically.
  - poorman 10y ago
    So it's like the old "merge", but doesn't create another commit when you close the PR?
    - morinted 10y ago
      Yes -- a merge merges one branch into another, and creates a merge commit while doing so. A rebase rewrites history, as if the PR had been written on master directly. There is no merge commit and no evidence of a previous branch. Some people prefer rebasing over merging because it gives you a linear commit history.
      - prophetjohn 10y ago
        This is not correct. What you're describing is two different styles of merging: fast-forward and no fast-forward, as indicated by the `--no-ff` and `--ff-only` flags provided to `git-merge` `git-rebase` will rewrite the history of the current branch. In this case, github will rewrite the history of the branch that you would like to merge such that the base commit (hence the name "rebase") or the commit that was branched off of is the latest commit on the default branch (usually master) Then when you merge it, you'll get a merge commit or not depending on whether or not you do a fast-forward merge
- whizzkid 10y ago
  I personally think that commit history should be kept as long as it makes sense. But in some cases, you don't want to keep some of the commit messages if they don't add anything to development flow. For example, - Force SSL flag when connecting to server - Use new syntax for SSL flag - Wrap flag implementation to its own method - Add forgotten tests This branch's main goal was to use ssl flag on external requests but ended up having several commits that does not really matter to other developers. Then you want to squash this branch into a commit and merge. Rebase and merge example, - Improve sorting by adding cache - Implement a separate config class to fetch caching times. - Modify search implementation to use cache config class These are rather important commits by themselves and better keep them separated in your history.
  - masklinn 10y ago
    > These are rather important commits by themselves and better keep them separated in your history. Which you could already do with a regular merge. Just because the commits are fine as part of a branch doesn't mean you want to keep them as part of mainline.
    - whizzkid 10y ago
      > Just because the commits are fine as part of a branch doesn't mean you want to keep them as part of mainline. Can you elaborate on this? I would be happy to learn more about it.
      - scrollaway 10y ago
        Parent is missing the point that you were making, which is the benefit of not squashing vs. squashing. What masklinn means is that when you just merge a PR, the history is retained as-is (same as with rebase). But instead of the history being flat, it's bumpy.
        masklinn 10y ago
        > Parent is missing the point that you were making, which is the benefit of not squashing vs. squashing. I'm not missing any point, I'm saying you already get that benefit with the original merge strategy.
mfontani 10y ago
Yay now it's only missing rebase then merge with --no-ff to be useful to me!
- deleted 10y ago
  [deleted]
- lmm 10y ago
  Your use case is super weird. If you're going to do a --no-ff merge why would you rebase?
  - tylorr 10y ago
    My guess is it keeps the history as linear as possible while still being able to undo the merge if necessary.
    - mfontani 10y ago
      Precisely correct, rebasing prior to merging ensures the branch about to be merged has no conflicts with the one being merged to, or that conflicts have been fixed in the branch prior to the merge… and merging with --no-ff ensures that the history of the branch gets kept "visually" when doing Git archaeology, as well as that the merge was "wanted"
      - eridius 10y ago
        > […] ensures the branch about to be merged has no conflicts with the one being merged to So does merging. > or that conflicts have been fixed in the branch prior to the merge Conflicts that show up in rebasing may not necessarily show up during a merge. Having to fix a bunch of conflicts that only occur because you rebased is tedious and introduces a source of potential error.
  - rchipman 10y ago
    It is useful for when you want the commits to be clearly grouped in the log (because they belong together conceptually), but you don't want to squash them all together (because, for example, you still want to be able to browse/bisect/revert some of those commits in the future). Also, it is easy to revert the merge commit if you want to undo the whole group at once.
dorianm 10y ago
I'm surprised there is no "commit tree" drawing to illustrate this. Basically it adds the commits to the master branch without a merge commit: e.g.: master: A - B - C feature: D - E master (after merge): A - B - C - D - E See: https://help.github.com/articles/about-pull-request-merges https://help.github.com/articles/about-pull-request-merges
- mfontani 10y ago
  Merging with --no-ff the just-rebased branch would've had the effect of adding a merge commit detailing what branch was indeed merged then.
- misnome 10y ago
  Personally I like the combined approach where the feature is first rebased (which I think should be done if possible because then it guarantees a clean commit with no conflicts) and then merged e.g. master: A - B - C feature: D - E post-master: A - B - C --------- F \- D - E -/ So in the view of the master the entire branch is one clean commit, but the branch history is still preserved.
  - masklinn 10y ago
    That would be more useful, as git log tools tend not to deal fantastically well with concurrently running branch, especially with interleaved side-branches (fork branch 1, fork branch 2, merge branch 1, merge branch 2, history looks like garbage in most Git tools)
    - geofft 10y ago
      You can get a lot better results with `git log -m --first-parent`, which effectively shows you only the merge commits, and shows all changes that were merged in as if they belonged to the merge commit. I have a `git log0` alias for this, as well as `git blame0` and `git show0`.
      - 10y ago
bnchrch 10y ago
Well this is a great addition! I've worked in places where `squash and merge` was standard procedure and others (including currently) where `rebase and merge` was the standard. It's nice to see the later finally getting some love so I don't have to keep going back to my own terminal after my PR is approved.
- ilkkao 10y ago
  Best part I think is that it's possible to make this the only allowed merge strategy for a repo.
- mkagenius 10y ago
  > so I don't have to keep going back to my own terminal after my PR is approved. Why would you have to go back to your terminal after the PR is approved? In either case, the merges are done by the admin..
  - bnchrch 10y ago
    This would be true if we had one person responsible for the merge! We have a small team of developers and proper rebase and merge is the responsibility of the PR author after it has passed code review + testing. In short: all devs are admins. edit: anyway this would still be a huge help to the sole admin if we had one.
  - ni-hil 10y ago
    Because sometimes the admin just tells you "ok, rebase then I will merge" and don't bother doing the rebase himself...
    - dsp1234 10y ago
      If there is a conflict during the rebase, the developer working on the new feature will have more information on how to fix it. So it makes sense for the developer to rebase before submitting.
  - lmm 10y ago
stormbrew 10y ago
This strikes me as perhaps the worst merge method possible for git. Sometimes people call your main branch soup, and this will make that an incredibly accurate description. People are bizarrely terrified of merge commits.
- yxhuvud 10y ago
  Then don't use it. I'd been happy to just have an option to support fast forward merges without merge commits, but this really nails my preferred work flow.
  - masklinn 10y ago
    > support fast forward merges without merge commits Isn't that essentially what this is? It's a rebase followed by a fast-forwarded merge, there should be no merge commit.
    - yxhuvud 10y ago
      Well, what I'm saying is that I'd be happy even if I had to rebase manually in the case that master had moved on since the pull request had been created.
- omouse 10y ago
  Indeed they are and you're going to get merge commits when you're continuing work on a branch and you're using branches rather than having people use separate repos (which is the superior choice). I'm amazed at how many companies allow branches rather than allowing repo forks.
- patrec 10y ago
  I, too used to wonder about this. But in git merge commits basically break bisect and revert. You can't tell git to only consider feature merges for it's bisection (which is normally what you want), nor will revert on a merge commit behave the way people expect. And not everyone enjoys reading long essays about to work around these problems.
robbles 10y ago
> Rebases automatically set the committer of the rebased commits to the current user, while keeping authorship information intact I don't understand this. The commits are rewritten - do they overwrite the author to the current user, or don't they? Or is there a distinction between "author" and "committer" I'm not aware of? Which one does blame use?
- cstrahan 10y ago
  Here are some resources for you: Difference between author and committer in Git? -- http://stackoverflow.com/questions/11856983/why-git-authordate-is-different-from-commitdate http://stackoverflow.com/questions/11856983/why-git-authorda... Why git AuthorDate is different from CommitDate? -- http://stackoverflow.com/questions/11856983/why-git-authordate-is-different-from-commitdate http://stackoverflow.com/questions/11856983/why-git-authorda... What does “authored 7 days ago; committed 14 hours ago” mean on GitHub? -- http://webapps.stackexchange.com/questions/70383/what-does-authored-7-days-ago-committed-14-hours-ago-mean-on-github http://webapps.stackexchange.com/questions/70383/what-does-a... I found all of that by googling "author vs committer git". The gist is that, yes, there is a distinction. If your friend, say, gives you a diff and you apply it with `git apply`, you're the committer and your friend is the author. EDIT: Regarding your other questions ("Which one does blame use?"): If you run $ man git-blame This is right at the top: NAME git-blame - Show what revision and author last modified each line of a file
pavel_lishin 10y ago
> Rebases automatically set the committer of the rebased commits to the current user, while keeping authorship information intact. The pull request's branch will not be modified by this operation. What does "keeping authorship information intact" mean? It says it resets the committer - when I'm going through `git log` or `git blame` output, doesn't this mean that I won't actually see the correct author? I don't want to have to dig through commit messages to get an accurate understanding of who wrote what.
- CUViper 10y ago
  Committer and author are tracked separately, and by default you only see the author. Try: git log --pretty=fuller
  - pavel_lishin 10y ago
    Ah, I didn't know that. Thanks!
- deleted 10y ago
  [deleted]
huntedsnark 10y ago
I really wish there was a fourth option: "Squash and Create a Merge Commit." It's nice to have a single commit per feature, but also see when it was merged into master and by who.
kevinsd 10y ago
This is great. On the other hand, no one mentioned Gitlab here yet as squash merge remains as an ee-only (paid-only) feature in Gitlab all this long. https://gitlab.com/gitlab-org/gitlab-ee/issues/150 https://gitlab.com/gitlab-org/gitlab-ee/issues/150 https://gitlab.com/gitlab-org/gitlab-ce/issues/4106 https://gitlab.com/gitlab-org/gitlab-ce/issues/4106
- asb 10y ago
  I'm sure it will enter CE fairly soon now. The Gitlab vs GitHub competition is good for all of us.
  - dmitry-k 10y ago
    Merge and rebase for pull requests might soon land in RhodeCode too.
- btashton 10y ago
  The inability to do rebase in CE is my biggest pain point with Gitlab right now.
tomstuart 10y ago
This is great, but as chrisseaton asked on Twitter [1], how does it interact with CI? The trees in the rebased commits are new and may never have been tested. Is GitHub going to expose a `refs/pulls/123/rebase` ref (cf. the existing `refs/pulls/123/merge`) for running through CI before the button can be pressed? EDIT: The docs [2] say: You aren't able to automatically rebase and merge on GitHub when: * Your pull request has merge conflicts. * Rebasing the commits from the base branch into the head branch runs into conflicts. * Rebasing your commits is considered "unsafe", such as when a rebase is possible without merge conflicts but would produce a different result than a merge would. So I guess the point is moot. You can’t rebase a branch unless the resulting tree is identical to the one you’d get from a merge, and that’s the tree that runs through CI. [1] https://twitter.com/ChrisGSeaton/status/780441251992731648 https://twitter.com/ChrisGSeaton/status/780441251992731648 [2] https://help.github.com/articles/about-pull-request-merges/#rebase-and-merge-your-pull-request-commits https://help.github.com/articles/about-pull-request-merges/#...
- spb 10y ago
  I had the same reaction: why isn't there a "rebase now" button, with a separate "merge" button you can click after CI etc. runs?
emodendroket 10y ago
Neat. They've been adding a lot of good stuff recently.

181 points by dwaxe 10y ago

poorman 10y ago

Can someone explain the benefit of this? vs. "squash and merge"?

crugej 10y ago
If you want to keep the individual commits rather than squash the into a single commit. This is useful if the author of the PR actually wrote good commits that you don't want to make into a single commit. I'm also thinking of the case where a squash could turn a series of commits into a single commit with a massive amount of changes that is harder to grok when going back historically.
- poorman 10y ago
  So it's like the old "merge", but doesn't create another commit when you close the PR?
  - morinted 10y ago
    Yes -- a merge merges one branch into another, and creates a merge commit while doing so. A rebase rewrites history, as if the PR had been written on master directly. There is no merge commit and no evidence of a previous branch. Some people prefer rebasing over merging because it gives you a linear commit history.
    - prophetjohn 10y ago
      This is not correct. What you're describing is two different styles of merging: fast-forward and no fast-forward, as indicated by the `--no-ff` and `--ff-only` flags provided to `git-merge` `git-rebase` will rewrite the history of the current branch. In this case, github will rewrite the history of the branch that you would like to merge such that the base commit (hence the name "rebase") or the commit that was branched off of is the latest commit on the default branch (usually master) Then when you merge it, you'll get a merge commit or not depending on whether or not you do a fast-forward merge
whizzkid 10y ago
I personally think that commit history should be kept as long as it makes sense. But in some cases, you don't want to keep some of the commit messages if they don't add anything to development flow. For example, - Force SSL flag when connecting to server - Use new syntax for SSL flag - Wrap flag implementation to its own method - Add forgotten tests This branch's main goal was to use ssl flag on external requests but ended up having several commits that does not really matter to other developers. Then you want to squash this branch into a commit and merge. Rebase and merge example, - Improve sorting by adding cache - Implement a separate config class to fetch caching times. - Modify search implementation to use cache config class These are rather important commits by themselves and better keep them separated in your history.
- masklinn 10y ago
  > These are rather important commits by themselves and better keep them separated in your history. Which you could already do with a regular merge. Just because the commits are fine as part of a branch doesn't mean you want to keep them as part of mainline.
  - whizzkid 10y ago
    > Just because the commits are fine as part of a branch doesn't mean you want to keep them as part of mainline. Can you elaborate on this? I would be happy to learn more about it.
    - scrollaway 10y ago
      Parent is missing the point that you were making, which is the benefit of not squashing vs. squashing. What masklinn means is that when you just merge a PR, the history is retained as-is (same as with rebase). But instead of the history being flat, it's bumpy.
      - masklinn 10y ago
        > Parent is missing the point that you were making, which is the benefit of not squashing vs. squashing. I'm not missing any point, I'm saying you already get that benefit with the original merge strategy.

mfontani 10y ago

Yay now it's only missing rebase then merge with --no-ff to be useful to me!

deleted 10y ago
[deleted]
lmm 10y ago
Your use case is super weird. If you're going to do a --no-ff merge why would you rebase?
- tylorr 10y ago
  My guess is it keeps the history as linear as possible while still being able to undo the merge if necessary.
  - mfontani 10y ago
    Precisely correct, rebasing prior to merging ensures the branch about to be merged has no conflicts with the one being merged to, or that conflicts have been fixed in the branch prior to the merge… and merging with --no-ff ensures that the history of the branch gets kept "visually" when doing Git archaeology, as well as that the merge was "wanted"
    - eridius 10y ago
      > […] ensures the branch about to be merged has no conflicts with the one being merged to So does merging. > or that conflicts have been fixed in the branch prior to the merge Conflicts that show up in rebasing may not necessarily show up during a merge. Having to fix a bunch of conflicts that only occur because you rebased is tedious and introduces a source of potential error.
- rchipman 10y ago
  It is useful for when you want the commits to be clearly grouped in the log (because they belong together conceptually), but you don't want to squash them all together (because, for example, you still want to be able to browse/bisect/revert some of those commits in the future). Also, it is easy to revert the merge commit if you want to undo the whole group at once.

dorianm 10y ago

I'm surprised there is no "commit tree" drawing to illustrate this. Basically it adds the commits to the master branch without a merge commit: e.g.: master: A - B - C feature: D - E master (after merge): A - B - C - D - E See: https://help.github.com/articles/about-pull-request-merges https://help.github.com/articles/about-pull-request-merges

mfontani 10y ago
Merging with --no-ff the just-rebased branch would've had the effect of adding a merge commit detailing what branch was indeed merged then.
misnome 10y ago
Personally I like the combined approach where the feature is first rebased (which I think should be done if possible because then it guarantees a clean commit with no conflicts) and then merged e.g. master: A - B - C feature: D - E post-master: A - B - C --------- F \- D - E -/ So in the view of the master the entire branch is one clean commit, but the branch history is still preserved.
- masklinn 10y ago
  That would be more useful, as git log tools tend not to deal fantastically well with concurrently running branch, especially with interleaved side-branches (fork branch 1, fork branch 2, merge branch 1, merge branch 2, history looks like garbage in most Git tools)
  - geofft 10y ago
    You can get a lot better results with `git log -m --first-parent`, which effectively shows you only the merge commits, and shows all changes that were merged in as if they belonged to the merge commit. I have a `git log0` alias for this, as well as `git blame0` and `git show0`.
    - 10y ago

bnchrch 10y ago

Well this is a great addition! I've worked in places where `squash and merge` was standard procedure and others (including currently) where `rebase and merge` was the standard. It's nice to see the later finally getting some love so I don't have to keep going back to my own terminal after my PR is approved.

ilkkao 10y ago
Best part I think is that it's possible to make this the only allowed merge strategy for a repo.
mkagenius 10y ago
> so I don't have to keep going back to my own terminal after my PR is approved. Why would you have to go back to your terminal after the PR is approved? In either case, the merges are done by the admin..
- bnchrch 10y ago
  This would be true if we had one person responsible for the merge! We have a small team of developers and proper rebase and merge is the responsibility of the PR author after it has passed code review + testing. In short: all devs are admins. edit: anyway this would still be a huge help to the sole admin if we had one.
- ni-hil 10y ago
  Because sometimes the admin just tells you "ok, rebase then I will merge" and don't bother doing the rebase himself...
  - dsp1234 10y ago
    If there is a conflict during the rebase, the developer working on the new feature will have more information on how to fix it. So it makes sense for the developer to rebase before submitting.
- lmm 10y ago

stormbrew 10y ago

This strikes me as perhaps the worst merge method possible for git. Sometimes people call your main branch soup, and this will make that an incredibly accurate description. People are bizarrely terrified of merge commits.

yxhuvud 10y ago
Then don't use it. I'd been happy to just have an option to support fast forward merges without merge commits, but this really nails my preferred work flow.
- masklinn 10y ago
  > support fast forward merges without merge commits Isn't that essentially what this is? It's a rebase followed by a fast-forwarded merge, there should be no merge commit.
  - yxhuvud 10y ago
    Well, what I'm saying is that I'd be happy even if I had to rebase manually in the case that master had moved on since the pull request had been created.
omouse 10y ago
Indeed they are and you're going to get merge commits when you're continuing work on a branch and you're using branches rather than having people use separate repos (which is the superior choice). I'm amazed at how many companies allow branches rather than allowing repo forks.
patrec 10y ago
I, too used to wonder about this. But in git merge commits basically break bisect and revert. You can't tell git to only consider feature merges for it's bisection (which is normally what you want), nor will revert on a merge commit behave the way people expect. And not everyone enjoys reading long essays about to work around these problems.

robbles 10y ago

> Rebases automatically set the committer of the rebased commits to the current user, while keeping authorship information intact I don't understand this. The commits are rewritten - do they overwrite the author to the current user, or don't they? Or is there a distinction between "author" and "committer" I'm not aware of? Which one does blame use?

cstrahan 10y ago
Here are some resources for you: Difference between author and committer in Git? -- http://stackoverflow.com/questions/11856983/why-git-authordate-is-different-from-commitdate http://stackoverflow.com/questions/11856983/why-git-authorda... Why git AuthorDate is different from CommitDate? -- http://stackoverflow.com/questions/11856983/why-git-authordate-is-different-from-commitdate http://stackoverflow.com/questions/11856983/why-git-authorda... What does “authored 7 days ago; committed 14 hours ago” mean on GitHub? -- http://webapps.stackexchange.com/questions/70383/what-does-authored-7-days-ago-committed-14-hours-ago-mean-on-github http://webapps.stackexchange.com/questions/70383/what-does-a... I found all of that by googling "author vs committer git". The gist is that, yes, there is a distinction. If your friend, say, gives you a diff and you apply it with `git apply`, you're the committer and your friend is the author. EDIT: Regarding your other questions ("Which one does blame use?"): If you run $ man git-blame This is right at the top: NAME git-blame - Show what revision and author last modified each line of a file

pavel_lishin 10y ago

> Rebases automatically set the committer of the rebased commits to the current user, while keeping authorship information intact. The pull request's branch will not be modified by this operation. What does "keeping authorship information intact" mean? It says it resets the committer - when I'm going through `git log` or `git blame` output, doesn't this mean that I won't actually see the correct author? I don't want to have to dig through commit messages to get an accurate understanding of who wrote what.

CUViper 10y ago
Committer and author are tracked separately, and by default you only see the author. Try: git log --pretty=fuller
- pavel_lishin 10y ago
  Ah, I didn't know that. Thanks!
deleted 10y ago
[deleted]

huntedsnark 10y ago

I really wish there was a fourth option: "Squash and Create a Merge Commit." It's nice to have a single commit per feature, but also see when it was merged into master and by who.

kevinsd 10y ago

This is great. On the other hand, no one mentioned Gitlab here yet as squash merge remains as an ee-only (paid-only) feature in Gitlab all this long. https://gitlab.com/gitlab-org/gitlab-ee/issues/150 https://gitlab.com/gitlab-org/gitlab-ee/issues/150 https://gitlab.com/gitlab-org/gitlab-ce/issues/4106 https://gitlab.com/gitlab-org/gitlab-ce/issues/4106

asb 10y ago
I'm sure it will enter CE fairly soon now. The Gitlab vs GitHub competition is good for all of us.
- dmitry-k 10y ago
  Merge and rebase for pull requests might soon land in RhodeCode too.
btashton 10y ago
The inability to do rebase in CE is my biggest pain point with Gitlab right now.

tomstuart 10y ago

This is great, but as chrisseaton asked on Twitter [1], how does it interact with CI? The trees in the rebased commits are new and may never have been tested. Is GitHub going to expose a `refs/pulls/123/rebase` ref (cf. the existing `refs/pulls/123/merge`) for running through CI before the button can be pressed? EDIT: The docs [2] say: You aren't able to automatically rebase and merge on GitHub when: * Your pull request has merge conflicts. * Rebasing the commits from the base branch into the head branch runs into conflicts. * Rebasing your commits is considered "unsafe", such as when a rebase is possible without merge conflicts but would produce a different result than a merge would. So I guess the point is moot. You can’t rebase a branch unless the resulting tree is identical to the one you’d get from a merge, and that’s the tree that runs through CI. [1] https://twitter.com/ChrisGSeaton/status/780441251992731648 https://twitter.com/ChrisGSeaton/status/780441251992731648 [2] https://help.github.com/articles/about-pull-request-merges/#rebase-and-merge-your-pull-request-commits https://help.github.com/articles/about-pull-request-merges/#...

spb 10y ago
I had the same reaction: why isn't there a "rebase now" button, with a separate "merge" button you can click after CI etc. runs?

emodendroket 10y ago

Neat. They've been adding a lot of good stuff recently.