generating diff between between two branches, after a partial review

Question

Let's say I have this situation in git:

  B---D---F---H---J---K--->topic
 /       /       /
A---C---E---G---I--->master

After getting as far as commit F, I submit the topic branch for review. The reviewer makes some suggestions, and the world moves on, so another merge is done. As answered here Git diff on topic branch, excluding merge commits that happened in the meantime? the way to generate the diff from A to F simply git diff master..topic. Now however, revisions B and D have already been reviewed, and I would like to exclude them from a new diff for review. git diff E..topic includes G and I however.

How do I get a diff of H, J and K, excluding things pulled in through G and I?

Answer 1

torek's suggestion (to do multiple reviews) is a possibility, but has the unfortunate side effect of meaning that code that was say in need of improvement after F and fixed in H must be reviewed twice: once in its sub-par state, and once when it has been finished. The nice thing about git diff master..topic was that it squashed these, and just gave the final difference between the branches.

What I did to solve the problem was create a branch from F, merge master into there, and then do a diff between my new temporary branch and the topic branch. That way all changes to master were glossed over, and only the changes to the topic branch since the previous review point are used.

Answer 2

Except for some special cases involving merge diffs (when you can use --combined), commit diffs are always pair-wise. This changes the way x..y gets interpreted. Instead of this meaning "all revs reachable from y that are not reachable from x", it means "compare rev x with rev y".

(It's worth noting here that the answer in the linked question glosses over the fact that x..y is interpreted differently in git diff than in other git commands.)

When you ask for E..topic, git resolves topic to a specific commit—in this case K—and then compares the tree in E to the tree in K. That's why you see changes from G and I, because those changes were brought in through J via the merge.

You could compare F and K, but this will still show the changes in G and I, because again, those changes are actually in J.

There is, in fact, only one way to ignore what happened in J while keeping F and H intact like that, and that is to build up a tree that either skips it, or reverts it. For instance, starting at H, you could add a patch based on the diff between J and K:

                K'        <-- HEAD[detached]
               /
  B---D---F---H---J---K   <-- topic
 /       /       /
A---C---E---G---I         <-- master

(to do this, git checkout topic~2 to get detached at commit H, then git cherry-pick topic to add, to your now detached HEAD, the changes made to go from J to K).

Now you can compare the tree in commit E with the tree in HEAD (at commit K'):

git diff E HEAD   # or   git diff E..HEAD   or   git diff E..

But that's not, in general, what people want to review. In fact, since you will almost certainly subsequently abandon rev K' entirely (generating it only for review purposes), they will be reviewing a version of code that is never used!

This is why the accepted answer to the linked question is to just git diff master..topic—which means exactly the same thing as git diff master topic, and in this case compares the tree for I with the tree for K. You are proposing to submit, on branch topic, a tree that looks like K. That's the code that will be in topic. It has some difference to the code that is in rev I in master. The difference it has to I is the sum of the changes in F, H, K, and any shim work that was done to make J fit in. It does have the changes made in G (and of course those in I itself) but those are not changes to I, those are merely changes incorporated in I.

A more thorough way to review the code is to present four diffs for review:

1. E vs F
2. F vs H
3. H vs J (perhaps a combined diff, H-and-I vs J)
4. J vs K

The first diff tells your reviewers that, if they accept it, it's going to be possible to check out a tree that looks like F (which is certainly true, just git checkout F). The second diff tells your reviewers that, having F in their repos, it's going to be possible to check out H (if they accept that), etc. Obviously four reviews are harder than one, which is why people accept short-cuts like comparing I vs K.