Using checklists for code review

We’ve all seen them, and most of us make them. Process lists, to-do lists, lists of all kinds–there’s something very satisfying about checking a little box or crossing an item off a list. However, checklists can serve a greater purpose than as a physical reminder that we’ve accomplished something. In many fields, checklists exist because human brains are fallable and prone to distraction. Pilots use checklists to make sure that amid dozens of relatively simple tasks, none get forgotten. Checklists used before and during surgery reduced the death rate by nearly 50%.

I’ve been doing code review for a long time. In the open source world, code review happens on pretty much any third party contribution. In my time at VMware, pretty much every non-trivial change was expected to be reviewed before checking it in to source control.

In general, people are pretty good at the code review process, but it’s sometimes surprising what can slip through. A natural consequence of the way our brains look at the world is that it’s easy to pay a lot of attention to small details and code style flubs, and completely miss the big picture. It’s for this reason that a lot of designers will start with rough sketches when they start gathering feedback–if something looks too finished, reviewers will only give them comments about the details. Unfortunately, unless you’re starting out with pseudocode or architectural sketches, source code almost always looks “finished” and can therefore encourage some bad habits.

One of the ways that I avoid the pitfall of giving superficial reviews is with checklists. I have a bunch of different sets of checklists that I’ve developed and tweaked over the past few years, and for any given change, I’ll select the sections which apply. I’ve applied some of these at VMware, working on a large desktop application code-base (mostly at the user interface layer), as well as with Review Board itself.

Here are some examples of some of the many checklists I use in my day to day work:

(more…)

Read More

RBCommons: New and Improved

Tonight, we deployed a major update to RBCommons that brings with it an improved look and feel and an assortment of new features.

RBCommons, as you may know, is powered by Review Board. Historically, we’ve used Review Board 1.6 under the hood, but now we’re on 1.7, the latest and greatest.

 

Shiny!

The first thing you should notice when you next log in is that there’s a cleaner, smoother feel to the site. Fewer sharp edges. More consistent font sizes. We’ve strived to bring more consistency and to shed a lot of our older warts. This will only get better from here on out.

As an example, look at how a review request used to look:

Old Look and Feel

Compared to how it now looks:

New Look and Feel

 

Improved Issue Tracking

It can be hard to keep track of all the issues your teammates want you to fix, especially if there’s a lot of reviews. Sometimes things just get missed. That would happen to us, at least, so we decided to fix it.

A summary of all opened issues is now shown right on the review request, making it easy to see how much work you have to do. You can filter the list or jump down to the relevant comments with one click.

Issue Summary Table

 

 

Moved Files in Diffs

If you’re using RBCommons with Git or Perforce, we’ll now show your moved files intelligently, instead of one big delete and one big add. That means you can move a file, make some changes, post it for review, and you’ll see those changes show up. Much easier to review!

 

Better File Attachments

We used to support uploading both screenshots and arbitrary file attachments, and you had to tell us which it was. Pretty ugly. It’s much simpler now. Just drag-and-drop your file onto the review request, and it’ll be attached. We’ll even show a preview of the file if we can (currently this supports images, MarkDown files, ReStructured Text files, and generic text-based files).

Just like before, you can review images, just like diffs. We’re going to be adding this ability for other types of files in the future.

 

Like it? Hate it? Have questions?

Let us know!

Read More