Code Review from the Command Line

January 13, 2018

I do the bulk of my code reviews from the command line, especially when reviewing larger changes. I’ve built up a number of tools and config settings that help me dig into the nuances of the code I’m reviewing, so that I can understand it better than if I were just browsing online.

In particular, I’ll walk through how I…

But first, let’s talk briefly about the point of code review in the first place.

Code review philosophy

When I ask that other people review my code, it’s an opportunity for me to teach them about the change I’ve just made. When I review someone else’s code, it’s to learn something from them. Some other benefits of code review include:

If this is different from how you think about code review, check out this talk. Code review is a powerful tool for learning and growing a team.

With that out of the way, let’s dive into the tools I use to maximize benefit I get from code review.

Checking out the code

The first step to reviewing code in the terminal is to check out the code in the first place. One option is to simply to git pull and then git checkout <branch>. But if you happen to be using GitHub, we can get this down to just one command:

hub pr checkout <pr-number>

It works using hub, which is a tool that exposes various features of GitHub from the command line. If the pull request is from someone else’s fork, hub is even smart enough to add their fork as a remote and fetch it.

At first glance

With the branch checked out locally, usually my next step is to get a feel for what changed. For this, I’ve written a git alias that shows:

Here’s the definition of git stat from my ~/.gitconfig:

[alias]
  # list files which have changed since REVIEW_BASE
  # (REVIEW_BASE defaults to 'master' in my zshrc)
  files = !git diff --name-only $(git merge-base HEAD \"$REVIEW_BASE\")

  # Same as above, but with a diff stat instead of just names
  # (better for interactive use)
  stat = !git diff --stat $(git merge-base HEAD \"$REVIEW_BASE\")

Under the hood, it just works using git diff, git merge-base, and a personal environment variable REVIEW_BASE.

REVIEW_BASE lets us choose which branch to review relative to. Most of the time, REVIEW_BASE is master, but this isn’t always the case! Some repos branch off of gh-pages. Sometimes I like to review the most recent commit as if it were its own branch.

To review the code relative so some other base, set REVIEW_BASE before running git stat:

# Review between 'gh-pages' and the current branch
REVIEW_BASE=gh-pages git stat

# Review changes made by the last commit of this branch:
REVIEW_BASE=HEAD^ git stat

I have export REVIEW_BASE=master in my ~/.bashrc, because most projects branch off of master.

Nothing too crazy yet—GitHub can already do everything we’ve seen so far. Let’s start to up the ante.

Visualizing file change frequency

I’ve written a short script that shows me a visualization of how frequently the files involved in this branch change over time:

This command identifies two main things:

Those two commands (git stat and git heatmap) are how I kick off my code review: getting a birds-eye view of the change and some historical context for what I’m dealing with. Next, I drill down into the relationships between the files that changed.

Visualizing relationships between files

At work I review JavaScript files, so I’ve built out this next bit of tooling specifically for JavaScript.1 It helps to understand which files import others, so I have a command that computes the dependency graph of the files changed on this branch:

This is where we start to see some distinct advantages over what GitHub provides. As you see above, the git depgraph alias calculates the dependency graph for files changed by this branch. Why is this useful?

In either case, we can see the structure of the change. Three files depend on Elements.js, so it’s serving the needs of many modules. Element.js only has one dependency, etc. Each branch’s dependency graph shows different information; it can be surprising what turns up.

I have the git depgraph alias defined like this:

[alias]
  depgraph = !git madge image --webpack-config webpack.config.js --basedir . --style solarized-dark src

Some notes about this definition:

The git depgraph alias is a game changer. It makes it easier to get spun up in new code bases, helps make sense of large changes, and just looks plain cool. But at the end of the day, we came here to review some code, so let’s take a look at how we can actually view the diffs of the files that changed.

Reviewing the diffs

To review the diffs, the simplest option is to just run git diff master..HEAD. This has a bunch of downsides:

My solution to all of these problems is to view the diffs in Vim, with the help of two Vim plugins and two git aliases. Before we get to that, here’s a screenshot:

Looks pretty similar to GitHub’s interface, with the added bonus that it’s using my favorite colorscheme! The Vim plugins featured are:

And to orchestrate the whole thing, I’ve set up these two aliases:

[alias]
  # NOTE: These aliases depend on the `git files` alias from
  # a few sections ago!

  # Open all files changed since REVIEW_BASE in Vim tabs
  # Then, run fugitive's :Gdiff in each tab, and finally
  # tell vim-gitgutter to show +/- for changes since REVIEW_BASE
  review = !vim -p $(git files) +\"tabdo Gdiff $REVIEW_BASE\" +\"let g:gitgutter_diff_base = '$REVIEW_BASE'\"

  # Same as the above, except specify names of files as arguments,
  # instead of opening all files:
  # git reviewone foo.js bar.js
  reviewone = !vim -p +\"tabdo Gdiff $REVIEW_BASE\" +\"let g:gitgutter_diff_base = '$REVIEW_BASE'\"

Here’s how they work:

Like with the git stat alias, these aliases respect the REVIEW_BASE environment variable I’ve set up in my ~/.bashrc. (Scroll back up for a refresher.) For example, to review all files relative to master:

REVIEW_BASE=master git review

At this point, you might think that all we’ve done is re-create the GitHub code review experience in Vim. But actually what we’ve done is so much more powerful.

Interactive Code Review

When reviewing on GitHub, the code is completely static—you can’t change it. Also, because the code is coming from GitHub’s servers, it’s laggy when you click around to view related files. By switching our code review to the terminal, we can now edit files, jump to other files, and run arbitrary commands at no cost.

It might not be obvious how huge of a win this is, so let’s see some examples. Take this screenshot of the requireElement function. It moved from above the findElement function to below it (probably because the former calls the latter):

But is the location of the requireElement function the only thing that’s changed? By editing the file to move the function back to its original location, vim-fugitive will automatically recompute the diff. And in fact, we can see that the type of the argument has changed too, from string to ElementType:

If we had been viewing this on GitHub, we might have taken for granted that the function didn’t change. But since we’re in our editor, we can interactively play around with our code and discover things we might have missed otherwise. The advantages of interactive code review go well beyond this example:

By having the full power of our editor, we can literally retrace the steps that the author went through to create the pull request. If our goal is to understand and learn from code review, there’s no better way than walking in the author’s shoes.

Recap

To recap, here’s a list of the tools I use to review code at the command line:

If you’re having trouble incorporating any of these into your workflow, feel free to reach out and let me know! I’m happy to help.


  1. The techniques here apply to any language that you can statically analyze. In particular, I have a rough prototype of everything JavaScript-specific you see here that works with Standard ML instead. If you can find me the dependency information for your favorite language, I’d be happy to help you turn it into a visualization.

  2. The open command is macOS-specific. On Linux, you might want to look at the display command from ImageMagick.

  3. I’ve patched the default Solarized colors for Vim so that lines retain their syntax highlighting in the diff mode, while the backgrounds are highlighted. You can see how this works in this commit: https://github.com/jez/vim-colors-solarized/commit/bca72cc

Read More

Concurrent Programming in ML: A Race

I want to call attention to what I think is a race condition in one of the code listings in the book “Concurrent Programming in ML”. The problem is that some of the data isn’t protected by a lock, which can lead to a stale read and incorrect behavior. I trace the bad behavior, and propose a fix. Continue reading

Prefer .then() over .catch()

Published on May 31, 2018

Union Types in Flow & Reason

Published on April 19, 2018