It’s generally good practice to rebase commits on a topic branch into a single commit before merging. It results in a much cleaner commit history, and makes rollbacks easier.

The Question

However, the question was raised: what happens if you…

• fix a bug (commit 1)
• create a Pull Request
• get feedback via the Pull Request
• fix the bug fix (commit 2)
• rebase those two commits together (new tree-ish)
• push that back to GitHub (requires push -f )

The answer is based on understanding that a GitHub pull request has two forms of commenting: * comments on the pull request itself * comments on the commits that the pull request encapsulates. These are the comments made on specific lines in the diff.

The Answer

When you rebase and then force that changed version of history up onto GitHub you are effectively throwing out the old commit (the old tree-ish no longer exists) and replacing it with a new one.

As a result any comments created on the diffs (and thus tied to a specific commit) will be lost, but the comments on the pull request itself will be preserved.

So, the proposed rule of thumb is to simply NOT rebase once you have made a pull request. Instead, just add commits to your topic branch. This will maintain the comment history for future people curious about this piece of code.

Alternate Solutions

There are alternate solutions, of course, but the consequences of them tend to not be worth it.

Rebase Anyway

Decide that you don’t care about maintaining the inline comments, then rebase and push -f back up to the server. The downside here is that at this point someone has already started looking at your code and has likely pulled it down. They will now be forced to blow away their old branch (or at least reset --hard back to a point in its history before your commit), which is a pain and not obvious unless you tell them directly.

Make A Second Topic Branch

You could close the first pull request after having gotten the set of commits to an approvable state, and then open a new pull request from a separate topic branch with all those commits having been rolled up into one. The problem here is that if your new pull request is merged in then git blame will point to the tree-ish of the new one and no-one will ever see the comments on the old pull request unless they search, but when they do they may find that the commits in the original request don’t exist in the codebase (they’re all under the rebased commit).