
Rebase and merge pull requests - dwaxe
https://github.com/blog/2243-rebase-and-merge-pull-requests
======
tomstuart
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/#...](https://help.github.com/articles/about-pull-request-
merges/#rebase-and-merge-your-pull-request-commits)

~~~
spb
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?

------
dorianm
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)

~~~
misnome
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
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
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`.

~~~
masklinn
> You can get a lot better results with `git log -m --first-parent`

Yes, however that only works in the CLI, not in any Git GUI I know of (though
GTK Bazaar log would pretty much do that, folding merge commits & following
the mainline by default).

------
bnchrch
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.

~~~
mkagenius
> 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..

~~~
ni-hil
Because sometimes the admin just tells you "ok, rebase then I will merge" and
don't bother doing the rebase himself...

~~~
dsp1234
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.

------
pavel_lishin
> _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
Committer and author are tracked separately, and by default you only see the
author. Try: git log --pretty=fuller

~~~
pavel_lishin
Ah, I didn't know that. Thanks!

------
kevinsd
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
I'm sure it will enter CE fairly soon now. The Gitlab vs GitHub competition is
good for all of us.

~~~
dmitry-k
Merge and rebase for pull requests might soon land in RhodeCode too.

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

~~~
lmm
Your use case is super weird. If you're going to do a --no-ff merge why would
you rebase?

~~~
jgraham
After being initially skeptical I was convinced by a colleague that this is a
good model.

The merge commit acts as a kind of "pushlog"; it tells you what actually
landed together as a single unit. It probably _also_ tells you what passed CI;
although many projects state that you shouldn't have individual commits that
don't pass CI it's rare that this is enforced below the level of the PR. That
should be good for bisection. Because the commits are always rebased onto
master (and of course you never allow merge commits other than from code being
integrated into master) your history is relatively clean, you don't get
multiple overlapping branches, but something like

    
    
      M1 ------- M2 -------- M3
       \ B1 - B2 / \ C1 - C2 /
    

Because the merges are --ff-only you don't get the confusing situation where
the merge commits themselves contain changes.

In principle this seems like the ideal way to use the tools git provides.
However I understand there are some minor rough edges e.g. with magic needed
to tell git bisect how to only try the merge commits.

~~~
lmm
> your history is relatively clean, you don't get multiple overlapping
> branches

Right, but what's the benefit of that? You still need to understand a history
with merges in, in which case it seems like you might as well get the safety
advantages of not rebasing.

~~~
wanderr
what are the safety advantages?

~~~
lmm
Rebasing tends to induce more conflicts than merging, tends to lead to force-
pushing which is often dangerous ( "\--force-with-lease" is safer but a lot
longer than "-f" ), and has to be done after PR review which bypasses one set
of checks particularly in the case where conflicts have to be fixed.

~~~
wanderr
presumably github will only let you rebase-merge if there are no conflicts. In
my experience though rebase tends to have more conflicts requiring manual
intervention vs merge, but that feels safer to me than merge occasionally
automatically resolving the conflict incorrectly.

Github also has protection to prevent branches from being forced, if you are
only forcing a feature branch with no collaborators, or at the end of
collaboration, it's not so bad, especially when git tells you the old hash you
just overwrote and you have the reflog, but again github's rebase-then-merge
should make that a non issue.

------
stormbrew
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.

~~~
scrollaway
Then please don't use it. In the mean time, here's how a flat tree looks to
contributors:

[https://github.com/jleclanche/fireplace/commits/master](https://github.com/jleclanche/fireplace/commits/master)

Versus a merge tree:

[https://github.com/hashicorp/terraform/commits/master](https://github.com/hashicorp/terraform/commits/master)

Consider that the git log is one of the first thing contributors will look at.
The cleaner it is, the better the experience.

I've been waiting for this for a long time. Thank you github!

~~~
barrkel
The merge tree looks much more useful me; links to the PR with discussion is a
lot nicer in a collaborative team than a series of commits by individuals.

~~~
scrollaway
What if your workflow primarily isn't PR-oriented? eg you have discussions on
a different channel than Github.

Point is, it's good to have both. And personally, I find terraform's git log
unreadable and unusable.

~~~
arielb1
I find terraform's git log to be more readable. You can get a quick overview
for which commits corresponds to which feature by following the first-parent
chain.

------
robbles
> 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
Here are some resources for you:

Difference between author and committer in Git? --
[http://stackoverflow.com/questions/11856983/why-git-
authorda...](http://stackoverflow.com/questions/11856983/why-git-authordate-
is-different-from-commitdate)

Why git AuthorDate is different from CommitDate? --
[http://stackoverflow.com/questions/11856983/why-git-
authorda...](http://stackoverflow.com/questions/11856983/why-git-authordate-
is-different-from-commitdate)

What does “authored 7 days ago; committed 14 hours ago” mean on GitHub? --
[http://webapps.stackexchange.com/questions/70383/what-
does-a...](http://webapps.stackexchange.com/questions/70383/what-does-
authored-7-days-ago-committed-14-hours-ago-mean-on-github)

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

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

~~~
whizzkid
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
> 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
> 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
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
> 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.

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

------
huntedsnark
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.

