interdiff-tooling.md

A colleague asked for code review, so I had a look. It was a small branch: a single commit affecting two files. Looking over the branch, I saw a common data extraction pattern emerging in unit tests, so suggested that a function would be easier to read and maintain than copied code. A variable name was a bit misleading, and I suggested a different name would be better. And I noted a spot where a class name was repeatedly hard-coded in the file.

I did all this through the Github UI from my phone, because mobile-first.

So, I requested three small changes, and after some small amount of time my colleague updated their branch. Rather than pushing the code review changes as a second commit to their branch, though, they instead squashed the second commit to the first one and did a force push. That's fine, I like a clean history too.

"Please have a second look," came the request. Since I was already up to speed on the initial delta, I figured it woudl be faster to read the difference between the two deltas using interdiff rather than re-read the second diff and do my own interdiffing in my head.

NAME
       interdiff - show differences between two diff files

DESCRIPTION
       interdiff creates a unified format diff that expresses the difference
       between two diffs. The diffs must both be relative to the same files.
       For best results, the diffs must have at least three lines of context.

So in order to get started, I need two diffs: the original commit, and the one that was force-pushed on top of it.

Since I'd done my initial code reviewing from my phone, I didn't have a copy of the git history that contained the overwritten commit. So in order to use interdiff, I'd need to track down the original commit and use that as one of the inputs to interdiff. Fortunately, our group chat room showed this message: develooper force-pushed branch topic/foo of repo-name from aaaaaa to bbbbbb.

First, fetch origin and check out the remote branch:

belden@localhost:repo$ git fetch origin
git checkout remote: Counting objects: 35, done.
remote: Compressing objects: 100% (32/32), done.
remote: Total 35 (delta 8), reused 0 (delta 0), pack-reused 0
Unpacking objects: 100% (35/35), done.
belden@localhost:repo$ git checkout origin/topic/foo
Note: checking out 'origin/topic/foo'.

You are in 'detached HEAD' state, blahblahblah.
HEAD is now at bbbbbbc... Foo some Bars

And now, cross my fingers that somehow git will show me the missing commit:

belden@localhost:repo$ git show aaaaaa
fatal: ambiguous argument 'aaaaaa': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions

Yeah, no joy. But GitHub still has the original commit: https://github.com/repo-name/commit/aaaaaa. And since the branch was small - just two files changed - I clicked through from the diff link to View each of the two files, and then on to the Raw link.

A little copy-and-paste to transfer the original delta's revised files to temp files on my machine. Then looking at the top of the GitHub diff view, immediately under the commit message, there's this note:

1 parent abcdef commit aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

Okay, now this is easy:

belden@localhost:repo$ git checkout -b temp abcdef       # recreate lost commit's branch point
belden@localhost:repo$ mv /tmp/lost-f1 lib/Path/To/File1  # restore first file from the lost commit
belden@localhost:repo$ mv /tmp/lost-f2 lib/Path/Of/File2  # restore second file from the lost commit
belden@localhost:repo$ git commit -a -m 'restore lost commit'

And now I can ask interdiff to do its work for me. Remembering the original GitHub notification of the force push: develooper force-pushed branch topic/foo of repo-name from aaaaaa to bbbbbb. My HEAD is now equivalent to aaaaaa, and the other half of my interdiffing will be bbbbbb.

belden@localhost:repo$ interdiff <(git show HEAD) <(git show bbbbbb)

This gave me the contents of the commit that got squashed away, and looking at that commit was much easier than re-reading the initial pull request all over again. In particular, it showed me that the things I had requested had been done, and also would have shown me any extra work that was also done. This second bit, of answering, "Okay, so you fixed the initial code review notes: did you add anything else in as well?" should be important to code reviewers.

Takeaways:

• this was only worth doing manually because the initial commit was so small, just 2 files
• tooling would make this much simpler
• absent tooling, force push is probably better reserved as a step post-code-review and pre-merge: otherwise we just waste our time re-reviewing
• exposing the squashed-away commits is also good for proving that the code-re-reviewer doesn't miss something new that was added

Todo: automate this, or find a tool that does. Given a GitHub pull request, it would be possible to find "outdated diffs" on it; automatically recreate commits that have been lost by force push; and produce an interdiff.

This not a screed against squash-and-force-push. I do that frequently for my own feature branches. This is a note that squash-and-force-push complicates code review a bit, and that we can make code review less complicated by either squashing at the very end, or by using/building tooling. I favor the tooling approach.

This answer on Stack Overflow would probably have worked for my colleague to recreate the lost diff, but didn't work for me since I never had a version of the branch in its initial state prior to the force push.