Why and How I Avoid GitHub’s Merge Pull Request Feature

I have used GitHub for years and there are many aspects of the service I’ve come to love. For example, per-project wikis, automatically closing issues via commit messages, the simple but straight-forward controls for managing repository access, and so on.

But there is one feature of GitHub that has always bothered me: merging pull requests. I want to explain why I don’t like the feature, why I often avoid it, and how I prefer to merge pull requests.

Problem One: Noisy, Unnecessary Commits

When you click GitHub’s button to merge a pull request it creates a commit like this, a merge commit. Inherently this is useful behavior. For example, when merging a branch that contains multiple commits—such as the commit I just referenced—I prefer to have a merge commit in the history; that commit becomes a useful target for git revert if it turns out that a branch of patches needs to be “unplugged” at a later date.

The problem, however, is when a pull request consists of a single commit. GitHub will still create a merge commit when you merge the pull request via its interface. For a pull request containing only a single commit this creates messy noise in the repository’s history. I don’t gain anything by having a merge commit in my master branch for the sake of integrating a single commit.

When it comes to merging a pull request containing a single commit, GitHub creates unnecessary noise.

Problem Two: Testing

Merging a pull request through GitHub does not allow me a sufficient amount of testing for the proposed changes. Services like Travis CI help a lot, but I still prefer to test pull requests locally. Here I think the convenient allure of merging a pull request with a single mouse-click degrades the quality and quantity of testing on a project. I’ll admit that this criticism is hypocritical coming from me, as I have allowed many bugs to creep into PHP Mode for Emacs.

Problem Three: Signing-Off and Other Commit Metadata

I like to sign-off on commits as an indication that I’ve personally reviewed and tested the patches in a pull request. To my knowledge this is not possible through GitHub’s UI. But I will admit that I could be wrong here, because due to the problem of testing mentioned above I’m already not using GitHub’s UI by the time I am ready to merge a pull request. So maybe there is relevant functionality here which I’ve overlooked.

There are times I also like to add lines to commit messages, such as references to relevant GitHub issues. Again, this isn’t possible from GitHub’s pull request UI as far as I know.

How I Prefer to Merge Pull Requests

When I receive a pull request it will be at a URL like this:

https://github.com/ejmr/php-mode/pull/255

The first thing I do is fetch the pull request, creating a new branch for it in the process. For the above I would do this:

$ git fetch origin pull/255/head:php7-syntax
$ git checkout php7-syntax

The general syntax for the fetch is pull/$ID/head:$BRANCHNAME. This example would grab the commits for the pull request at that example URL and put them into the new branch php7-syntax. Which I then checkout and begin to review and test.

When I’m satisfied with the pull request and am ready to merge I will do one of two things. In both cases I will start with git checkout master.

If the pull request contains only a single commit then I will run git cherry-pick -s $BRANCHNAME. This signs-off on the commit.

If the request contains multiple commits then I do this:

$ git merge --no-ff --no-commit $BRANCHNAME
$ git commit -sv

This creates a sign-off on the merge commit, which is my personal preference for indicating I reviewed everything in the branch. And it also gives me a chance to add any important information to the merge commit if necessary.

In both cases I will end up with a different SHA-1 hash for the result than if I had used GitHub’s feature to automatically merge the pull request. Because of that I always paste that SHA-1 into a message on GitHub when I am closing the pull request. That way the author of the pull request can see that their work is merged and thus can delete their own branch if they want.

Conclusion

GitHub’s feature to merge pull requests does not give me the level of control that I want during a merge process. That is not to say the feature is worthless or anything. It just doesn’t fit into how I prefer to manage a repository.

If you like that feature of GitHub I would love to hear why, or if you’re like me and prefer to avoid it then I’d also love to hear about your own process for merging pull requests.

Advertisements

3 thoughts on “Why and How I Avoid GitHub’s Merge Pull Request Feature

  1. Hello Eric,

    Yesterday I shared a link to this article to my developers fellows at JustPark and it immediately fired up a very interesting technical conversation.

    As far as Problem One is concerned I would very interested in knowing in general how do you feel about applications that write commit data to the repo under control.

    With Problems Two and Three I completely agree, I’m always trying to convince everyone that it’s never good to trust too much automation, not even trusting a full battery of passing tests.

    Some of my colleagues could be right when they say I’m a bit polarized when it comes to the command-line, but I also enjoy the possibility of signing off commits with my private key, although I don’t think I’ve ever done this at work.

    The conversation carried on towards the lack of conflict markers in the GitHub pull request UI, and how this sometimes encourage developers to embark the quest of solo conflict resolutions, possibly causing the loss or misplacement of important lines of code.

    Code reviews are time consuming and GitHub really simplifies them, a bit too much. I have found that Differential’s code reviews [1,2] are much better alternative for teams of PHP developers.

    One of my colleagues shared this article [3] which does a fantastic job at explaining my long unexplained fear for the possibility of logical conflicts getting resolved cleanly and causing all sorts of WTF moments, if and when they get found.

    To be fair I don’t think this is a problem with GitHub itself but with Git, or well with its users myself often included, who tend to forget that it’s meant to be used like a graph not as a linear timeline.

    I could be wrong but I feel that this problem is often exacerbated when branches are frequently rebased.

    Even then, one cannot pretend any diff algorithm to do as good job as a programmer.

    1 http://phabricator.org/applications/differential/
    2 https://secure.phabricator.com/book/phabricator/article/arcanist/
    3 https://developer.atlassian.com/blog/2015/01/a-better-pull-request/

    1. Heyo friend! Thanks for spreading the article.

      As far as Problem One is concerned I would very interested in knowing in general how do you feel about applications that write commit data to the repo under control.

      Personally I am a control freak when it comes to Git repositories and source-control management in general. With Git specifically, since it is often impossible to rewrite history once it’s made public, I do not like anything which may commit data or information automatically which may slip by my notice. I do not feel GitHub is intrusive in this regard, except when it comes to the merging of pull requests which I described. Reading the history of Git repos is a large part of my workflow so anything that creates needless, “noisy” commits is something I go to great lengths to avoid.

      With Problems Two and Three I completely agree, I’m always trying to convince everyone that it’s never good to trust too much automation, not even trusting a full battery of passing tests.

      Yeah, I don’t think one can ever have too much testing on a project. A specific problem I deal with is the game I’m working on, which my team and I share across a private GitHub repository. Even though we have a test suite and are using services like Travis-CI, a game is a type of application for which I think it’s impossible to truly test automatically. Hence I have never used GitHub’s feature to merge pull requests on that project, because it gives me no chance to actually play the game with the proposed patches in place, the best (and often only) way to test any changes.

      To be fair I don’t think this is a problem with GitHub itself but with Git, or well with its users myself often included, who tend to forget that it’s meant to be used like a graph not as a linear timeline.

      I could be wrong but I feel that this problem is often exacerbated when branches are frequently rebased.

      I would agree with you that the best use of Git is not to think about the repository as a linear timeline, similar to my experiences with Subversion; despite supporting branches, every Subversion project I ever worked on just used a single ‘master’ branch for everything, and that led to all kinds of problems. I have often felt that programmers coming from tools like Subversion and CVS (like myself) have difficulty either seeing the value of branches and adapting to making them part of their workflow; it certainly took me sometime to grow accustomed to it. And I don’t feel like Git itself does a great job of selling people on why using branches and not constantly rebasing them is a good practice and what benefits it provides.

      Even then, one cannot pretend any diff algorithm to do as good job as a programmer.

      I completely agree. On that note, I’m curious to someday try out this tool:

      https://www.semanticmerge.com/

      Finally, thanks for the links to read!

Add Your Thoughts

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s