One question that we see pretty often is how to create an engineering culture that embraces code review. It’s hard to overstate the benefits of code review, and engineering groups who aren’t doing it are missing a huge opportunity to improve the quality of their software, as well as get a lot of other benefits.

Jeff Atwood of Coding Horror summarized a lot of these benefits in his article Code Reviews: Just Do It. His only caveat was “The only hurdle to a code review is finding a developer you respect to do it”. If you’re a one-person shop, then that’s a very hard question, but most of us work with developers we respect: our coworkers.

Unfortunately, getting engineers to change their workflow and processes is a lot like herding cats: generally impossible, except in the case that it makes their lives easier (and then they’ll herd themselves). One of the best managers I ever had explained to me her observation that people only change if they want to, and so the best approach to effecting change is to work on making them want it.

The extensive benefits of code review make it an easy sell, but if it’s not introduced in the right way, it can create tension and resentment, which will kill any chance of adoption.

The easy answer to creating a code review culture is to build it in from the start. Many of the startups that I’ve talked to recently have incorporated some form of code review from day one. Fortunately, a lot of new companies these days are founded by people who have worked in an environment where robust code review is a given, and when they strike it out on their own, it’s just the most natural way to do things. If you’re forming a new organization (or even a new group within an existing organization), it’s the ideal time to build in good practices from the start.

Where it becomes a sticky, difficult question is when dealing with an established company with entrenched practices. How do you get people to change the way they work? Anyone who has ever worked with actual humans will know that bursting in the door and announcing a new process is liable to be met with groans, laughter, or silent resentment (and often all three).

Sneak it in through the back door

Joel Spolsky has a lot of advice on how to get things done when you’re only a grunt, and a lot of that advice works very well for adding code review to existing processes. Going back to the psychology of change, it’s easiest to ask people to do code reviews for you, rather than the other way around. Dropping in on a coworker and asking them to take a look at your code before you check it in is received much better than dropping in on them and saying you’d like to have approval power over what they’re doing.

When I started working on the Workstation team at VMware, we already expected code reviews for every major change, but the official system used email. After creating Review Board, Christian Hammond and I wanted to use it at work as well as home, and we set up a server for ourselves. For the first couple weeks, we just sent things back and forth to each other, while still using the old process for everyone else. As the benefits of improved tooling became visible to others, it took off, and before we knew it, we had people we’d never even met using it.

If you’re a manager, don’t force it

For those of you who might be reading who are managers of software engineers, and want to add code review to your development process, I don’t have a ton of advice, because I haven’t done a whole lot of management. The biggest thing I can urge is to go slow and avoid prescribing any particular behavior.

Educate your employees, maybe set up some tools, and then try to let things happen naturally. Making code reviews a part of the way you measure performance is a nice goal, but doing so when people are still trying to figure it out incentivizes the wrong behavior. The more structure and process that’s enforced at the beginning, the less likely that code review will actually take root, or if it does, it will be a cursory process to get the rubber stamp.

Always remember that it’s not about ego

Ego in code reviews is a big enough topic that we’ll probably tackle it in a separate post, but I think it’s also worth mentioning here. When people are new to code review, it can feel like a brutal, ugly process. It kind of sucks to expose your hard work and have people tear it apart. A healthy code review culture requires respect on all sides–people reviewing code need to remember that there’s a human behind the code, and people asking for code reviews need to remember that they are not their code, and it’s only through collaboration that we produce our best work.

Read More

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