One thing we should agree on is how to merge pull requests. This blog post presents the three alternatives: http://differential.io/blog/best-way-to-merge-a-github-pull-request.
The first option is clearly the easiest since all you have to do is click GitHub's merge button, but I personally find that the merge commits pollute the history (particularly if only one or two commits are being merged). I know João disagrees with me on this matter.
It's pretty easy to implement the other workflows with a couple of commands, something like:
# see https://help.github.com/articles/checking-out-pull-requests-locally # on an up-to-date repo $ git checkout pr/66 $ git rebase origin/master $ git push origin pr/66:master
Or using hub (http://hub.github.com):
# on an up-to-date master $ hub am https://github.com/slime/slime/pull/66 $ git push
I'm partial to the second option listed on the blog post, but I have very few commits in SLIME so my opinion should weight very little. :-)
Cheers,
Luís Oliveira luismbo@gmail.com writes:
The first option is clearly the easiest since all you have to do is click GitHub's merge button, but I personally find that the merge commits pollute the history (particularly if only one or two commits are being merged). I know João disagrees with me on this matter.
I don't think of merge commit objects as pollution. At least in the particular "Merge-button" workflow, in other cases it is indeed problematic, but these are not relevant here.
The "merge" button also has these advantages:
* it is very familiar in the github ecosystem, and github passers-by will find it familiar (as they already have apparently).
* It makes for a very pretty and readable "network" graph
https://github.com/slime/slime/network
* It makes the history on https://github.com/slime/slime/commits/master accurately reflect the pull requests merged, and link to their respective discussion threads.
I've confirmed that I when tried out Luís's rebase technique for https://github.com/slime/slime/pull/73, we didn't get the last two advantages. Plus, as Luís says, it is more complicated for git newbies, and requires a separate step to close the pull request.
So yes, I'm partial to the first, but won't kill myself over it, particularly for small changes as you say. For big changes I might cut a wrist or two :-)
João
On Mon, Jan 6, 2014 at 11:07 AM, João Távora joaotavora@gmail.com wrote:
I don't think of merge commit objects as pollution.
Here's an example of the noise I was refering to: http://i.imgur.com/St9mrtl.png
On GitHub's commit list (https://github.com/slime/slime/commits/master) it's confusing for other reasons: there is no visible relation between the merge commits and the commits that have been merged.
Luís Oliveira luismbo@gmail.com writes:
On Mon, Jan 6, 2014 at 11:07 AM, João Távora joaotavora@gmail.com wrote:
I don't think of merge commit objects as pollution.
Here's an example of the noise I was refering to: http://i.imgur.com/St9mrtl.png
Actually looks kinda cool :-) and gitk could do a better job of rendering that graph if it wanted. The network graph I linked displays the very same information (even more!) in a manner which you will probably find less noisy.
I think we can agree looks and readability are subjective. Let's not turn this into another rebase/merge thread, decide on one, and move on. I'm +1 to clicking the "Merge" button.
Hi,
I am not a contributor to SLIME (just a user), but I do use git quite heavily. I agree with João that merge commits don't pollute, but rather, help understand the commit history better. Besides what João mentions, there's atleast one other advantage to retaining merge commits.
If you use pull requests/branches for everything (features, bugs -- even tiny ones), the one advantage that merge commits give you is that it makes very easy to get a list of things that changed since the last release (to update the changelog, for example):
git log --merges <last-release-tag>..HEAD
Writing HEAD above is optional, you could just use:
git log --merges <last-release-tag>..
You can refine this further. For example, if you just want to see merges which were done via the "Merge" button on github, you could do this:
git log --grep="Merge pull request" --merges <last-release-tag>..
Also, there's one issue with rebase which you'll not encounter very often, but when you do, it can be a pain. Suppose a few commits in your branch affect one particular region of your file. And in master, an unrelated change affects the same region (e.g. someone fixed whitespace or formatting, or changed names). Now both git-merge and git-rebase will result in conflicts. The difference is, with git-merge, you only need to manually resolve the conflicts once. With git-rebase, you will need to resolve the conflicts once for nearly every commit in your branch affecting that region.
For this reason, I have a neutral opinion of git-rebase. Its ability to cleanup commit histories is great and I use it in my feature/bugfix branches when syncing them with master, but if there are conflicts (or if my changes are already pushed to remote[1]), I just use `git merge master`.
FWIW, I have never felt that merges pollute commit history, because when I don't want to see merge commits, I can easily make them disappear:
git log --no-merges gitk --no-merges
Just my 2 cents.
Chaitanya
1. Its worth reading Linus's advice on when you should and shouldn't use git-rebase: http://www.mail-archive.com/dri-devel@lists.sourceforge.net/msg39091.html
On Tue, Jan 7, 2014 at 12:15 AM, João Távora joaot@siscog.pt wrote:
Luís Oliveira luismbo@gmail.com writes:
On Mon, Jan 6, 2014 at 11:07 AM, João Távora joaotavora@gmail.com wrote:
I don't think of merge commit objects as pollution.
Here's an example of the noise I was refering to: http://i.imgur.com/St9mrtl.png
Actually looks kinda cool :-) and gitk could do a better job of rendering that graph if it wanted. The network graph I linked displays the very same information (even more!) in a manner which you will probably find less noisy.
I think we can agree looks and readability are subjective. Let's not turn this into another rebase/merge thread, decide on one, and move on. I'm +1 to clicking the "Merge" button.
-
Chaitanya Gupta -
joaot＠siscog.pt -
joaotavora＠gmail.com -
Luís Oliveira