After a long time spent in management, I found myself switching back to an IC role in February. Since then, I’ve spent more time than I’d care to admit re-learning how to do all the things I used to do. Or…maybe not re-learning outright, but rebuilding habits and patterns that used to be automatic, but that I now have to think about.

One of those patterns involved code reviews. The place I work has a bias towards large, epic-sized PRs. If it’s been a minute since I’ve been an IC, it’s been an hour since I was actively using GitHub for reviews, and an aeon since I was reviewing truly massive PRs. While I was pleasantly surprised to find that GitHub has improved a lot in that area, it’s got a long way to go. You can mark a given file as reviewed, for example, but not a given chunk. You can’t really make notes to yourself in any way other than leaving draft comments, which means you need to be careful what and how you write notes lest you accidentally publish a comment saying, I dunno, “oy, we’ve got to fix Suebob’s dependency so this isn’t necessary”. And while you can zoom out to see a whole file instead of just the changes, you can’t easily navigate around the code base to understand how the PR fits into the larger whole.

Thankfully, I keep a developer’s journal where I write down the various problems I’ve hit over the years and how I’ve approached them, so I quickly banged out hsh q review and found my notes on doing large PRs. While not all of them holds up (and some of them are specific to dealing with the vagaries of Phabricator and Gerrit), there’s enough there that’s useful that I figured it’d be worth sharing.

Pull the changes locally

First things first: the best tool to understand code is the editor and tooling you use every day. If it’s a really trivial change, I’ll review it in GitHub directly, but otherwise, it’s almost always worth pulling down the changes locally and opening them up in a relevant IDE or editor. Once I’ve done that, I’ll see any warnings and suggestions that my environment offers, I’ll be able to use tooling like “find references”/“find overrides”/etc. to make sure that I understand the full impact of the change, I can run (or even debug, step through, or alter!) the code and tests, and so on. This sets me up in a much better place to understand what’s actually happening with the PR, and to verify it works as expected.

GitHub makes it really easy to grab changes locally with their CLI tool (it’s just gh co <PR number>), but once you’ve done so, you’ll note that git diff shows nothing: there are no changes to look at because you’re up-to-date on the feature branch. If you don’t do anything else, you’ll either need to keep the GitHub review open in another tab, or you’ll need to keep the output of git diff main.. handy in another terminal window so you know what you should actually be looking at.

Thankfully, there’s a much better way.

git reset --mixed for fun and reviews

There’s a really easy trick to get PRs’ changes locally in such a way that Git itself, as well as your editor and tooling, will show the changes. If you’ve ever messed up a merge, or wanted to throw out your work, you’ve probably used git reset --hard main, which discards all of your local changes and sets you back up on the trunk. But a great tool in this case is to use is git reset --mixed (a.k.a. the vanilla git reset, without the --hard) in the form of git reset --mixed main, which will leave the changes in your working copy, but reset both the index and HEAD (i.e., what commit you’re working off of). Once you’ve done that, you’ll see that you now have all of the PR’s changes showing in git diff, ready to be staged. In many editors (including Visual Studio Code and Helix, the two I use most often), you can even jump from change to change across the project, making it really easy to see what was altered and how those changes fit into the project as a whole.

I use this pattern enough that I have a fish function, revpr, for…well, “review PR”:

function revpr --description "check out a given PR for review"
  gh co $argv[1]
  git reset --mixed main
end

This is already a huge improvement: we can see what’s changed in our preferred tooling, as well as navigate around, debug, and experiment. But we haven’t solved leaving comments or tracking what we’ve viewed. Let’s solve those one at a time.

Stage hunks you’ve reviewed

One of the reasons I use git reset --mixed instead of something like git reset --soft is because it resets the index. People like the index in the first place is because it lets you stage parts of a change, rather than just unceremoniously committing all the changes at once. git add -p and similar tools even let you add just parts of a file to a commit. We can use that for our own purposes: in this case, if you add hunks as you review them, you can easily see what you’ve already looked at via git diff (stuff yet to be reviewed) versus git diff --staged (stuff you have reviewed). In many editors, including again the two I use, staged changes are shown differently than unstaged changes, so I don’t even actually need to jump to the CLI to track where I am in the review.

Keep comments in the code

That just leaves where to put comments. My suggestion here is simple: put the comments in the code itself. I use two specific forms for my comments to make them easy to find: // REVIEW(bmps) are comments I want to put in GitHub, and // NOTE(bmps) are comments just for myself.[1]

Now the one part that’s annoying: once I’ve concluded the review, I find all of those review comments (you can use any global find you want; I have a fish function that just does rg -F 'REVIEW(bmps)') and…erm, well, manually copy them into GitHub.

I’ll be honest: this is one part of the process that genuinely annoys me. I’ve looked at tools like prr for inspiration on how to automate things, but I haven’t had any “eureka!” moment on how to do this without human intervention. I will say that the ability to write review notes to myself however I want, knowing that I’ll have a chance to manually review and edit everything before sharing with the author, does have some pretty strong benefits—but so would just having a sort of review-your-review-by-hunk flow similar to git add -p. That all said, this annoyance is completely worth it to me due to all of the other benefits. It’s just not where I want it.

Suggestions on improving this part are always welcome in my inbox.

Use a separate Git worktree for reviews

Finally, one tip I that’s new to me as of this year (and really the past few months). All of the above has one major drawback (at least, if you’re not using something like Jujutsu), which is that you have to either stash your in-progress changes or throw them into a temporary commit before you review work. Yes, you can make an entirely separate clone of every Git repo for reviews, but 1) that can eat a lot of disk space (one of the repos I work in most often is a whopping 8.6 GiB), and 2) you have to constantly remember both to grab what you’re reviewing, and to update main, or else the git reset --mixed step will result in you seeing a bunch of false changes.[2]

The fix for both is to use a git worktree specifically for reviews. If it’s a repo I’m not in much, I usually won’t bother with one, and will just do the review right in the tree. But otherwise, a simple git worktree add -d ../foobar-reviews gives me a clean place to work whose branches will always be fully in sync, but that won’t mess with my own in-progress code.

Future improvements

I’ve already called out manually copying comments over to GitHub as annoying and something I want to improve, but there are three other bits I’ve not been thrilled with.

First, I haven’t really found a good way to keep my comments locally as the review matures. With the above flow, from Git’s perspective, you’re basically squashing the entire PR, which puts you into a prime position to destroy the upstream PR and/or push up your inline comments. I’ve never gotten as far as actually doing that, but I’ve gotten uncomfortably close. I’m currently toying with a variant of the above where I make a branch to hold the review comments right when I start, and then squash/commit on that, but that ends up feeling like a lot of ceremony for relatively few benefits.

Regardless of which of the above approaches I pick, the following git rebase when there are upstream changes inevitably results in merge conflicts as my notes are addressed. This is actually not a huge deal, since the conflicts will by definition be just with my review comments and thus be trivial to resolve, but it can sometimes feel like more friction than I’d like. I feel as if a custom merge tool would probably handle this well, but I’m still chewing on how exactly that merge tool would work and what it would actually do.

Finally, I can’t easily see what comments others have made.[3] In practice, this isn’t that annoying: I like to go through a review first without looking at what others have said and only then read their comments anyway, so it’s not a huge deal to do my review pass locally using the above workflow and then read GitHub afterwards, but I definitely feel like that sequence could be improved.[4]

That all said, for me, this flow ends up working very well in practice, and is a meaningful improvement on the vanilla GitHub PR flow for large changes. If you find yourself in a similar boat, I think this approach is worth a try.

  1. I didn’t used to bother with the // NOTE(bmps) form, instead just doing plain comments for those, but I realized after awhile that my notes often evolve into review comments over time, so I started marking them separately so they’d be easier to find. ↩︎

  2. Strictly speaking, git reset --mixed origin/main works as long as you’ve done git fetch, which you have to do to grab the PR anyway, but getting that muscle memory right is tricky. ↩︎

  3. This is a problem that prr also suffers—which is really unfortunate, since having a mailing-list-style review is perfect for this exact case. I ultimately concluded that prr’s approach just isn’t one that appeals to me, but if I’d decided to stick with it, trying to add that functionality would’ve been a fun contribution. ↩︎

  4. It’s worth noting that Visual Studio Code can show GitHub review comments directly in the editor via the GitHub extension. If I used Visual Studio Code as my sole and primary editor, I’d be pretty tempted to look at that route more closely, but since my preferred editor is Helix, and I also spend a ton of time in PhpStorm, I prefer a workflow that works identically in all three places. ↩︎