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.
Improve this question
5
  • 1
    Can you describe some benefits and drawbacks you see in doing the process this way?
    – Alex Feinman
    Mar 26, 2014 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, 2014 at 21:31
  • 1
    Nothing wrong with setting standards. Just make it clear upfront what is expected. Example: symfony.com/doc/current/contributing/code/patches.html Scroll down to Step 3: Submit your Patch
    – Cerad
    Mar 26, 2014 at 21:43
  • 6
    @Granko: "Rebase" and "rebase into a single commit" are two separate issues.
    – Matthew Scharley
    May 2, 2014 at 0: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 1, 2016 at 15:22

1 Answer 1

Reset to default
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.

Improve this answer
2
  • +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 2, 2014 at 0:53
  • 1
    +1 for actually providing an answer and not just comments!
    – Pablojim
    May 2, 2014 at 13:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.