25

I maintain a relatively popular github repo.

When a pull request is good to merge I usually ask the author to rebase it to a single commit before I merge it (especially when there have been multiple small edits).

Is this good git practice? Is this acceptable/standard GitHub etiquette?

So some benefits:

  • I get a nice clean commit history in the commit logs
  • I don't need to alter the commit myself
  • It delegates some of the work

Some possible drawbacks:

  • I'm not sure if this is good etiquette
  • I'm not sure if this is a good git practice
  • I've usually already asked for a few other changes - this is one more and I don't want to discourage contributors.
Pablojim
  • 359
  • 3
  • 5
  • 1
    Can you describe some benefits and drawbacks you see in doing the process this way? – Alex Feinman Mar 26 '14 at 18:08
  • 1
    Some additional benefits and drawbacks worth considering. good: git-bisect and other reversals are easier when every commit produces a buildable or otherwise complete state, and this approach is a simple way to guarantee that. bad: small changes with simple commit messages get rolled into mega commits. E.G. "Changed this one line to fix corner case so-and-so" might get rolled into "Adding feature foo, *big list of changes*". This makes finding the reason for a specific change a bit harder. – Gankro Mar 26 '14 at 21:31
  • 1
    Nothing wrong with setting standards. Just make it clear upfront what is expected. Example: http://symfony.com/doc/current/contributing/code/patches.html Scroll down to Step 3: Submit your Patch – Cerad Mar 26 '14 at 21:43
  • 6
    @Granko: "Rebase" and "rebase into a single commit" are two separate issues. – Matthew Scharley May 02 '14 at 00:37
  • 2
    If a contributor is asked to do this, should they overwrite the branch of the pull request with `git push -f`? – Flimm May 01 '16 at 15:22

1 Answers1

16

As far as Git is concerned, it's something of a holy war whether you should simply merge branches or rebase commits on the latest version of the branch you are merging into. There's plenty of conversations about which is better if you do a quick search on Programmers.SE.

As to the etiquette behind it, lets deal with this from a practical perspective. When dealing with new code coming from someone else it's always best to get them to either merge the latest changes from the branch or freshly rebase it before merging to ensure a clean merge. Remember, they wrote the code so they are usually by far the most qualified to deal with any merge/rebase conflicts. I personally don't see a problem with it, and see this request all the time from other people. For me, if there's no conflicts then I'll often just do it myself since it's a two second update that git can apply itself. But if there's conflicts then I will always ask the original author of the code to deal with it themselves.

Also, for GitHub (at the minimum) specifically, they will display a link to your CONTRIBUTING file above any PR attempts so that makes a good place to outline your expectations and many projects do include that they will only merge up to date branches.

Community
  • 1
Matthew Scharley
  • 1,627
  • 13
  • 17
  • +1 for bringing pragmatism into the discussion. Yes, that's the whole point of it. It can be quite hard to resolve complex conflicts in large pull requests, especially when a certain number of commits are involved. That's the point where the original author should be asked to chime in. The easy conflicts are not the issue, they never were and never will be. – JensG May 02 '14 at 00:53
  • 1
    +1 for actually providing an answer and not just comments! – Pablojim May 02 '14 at 13:09