It’s nearly midnight. The end of a long coding session. Your code seems to work pretty well; all tests pass and it doesn’t blow up when you use it. It was a hard-fought battle, and you’re feeling great! The only reason it’s not already in the product is because your team has fostered a culture around code review.

At this point, you may be feeling ready to just post your code for review and move on to something else, waiting for the feedback to roll in. Is that what you do? Well, stop it! You’re only making things harder for yourself.

Your first reviewer should never be another person. It should be you!

Self-review saves time and increases quality

A typical code review cycle can easily take anywhere from an hour to a couple days. If your code isn’t perfect and your team is thorough, you will go through at least one cycle with at least one developer.

Likely more, since developers are often distracted by the tiny problems (style issues, typos) and ignore the larger problems (algorithmic issues, design flaws, bigger picture tasks) when reviewing. Oh they’ll get to the larger ones, but it might take another full review cycle or two.

Self-review can save you at least one cycle. Maybe more. The other reviewers will get to spend more time focusing on the more interesting issues instead of the little flaws, making them more likely to want to review the code. Plus, it’s far easier to convince people to look at your code when they think they’ll get through it quickly.

Let’s summarize the benefits:

• Your code lands faster.
• The feedback is more useful.
• People will think you’re awesome for not wasting their time and will bake you cookies as thanks. Probably.

Now let’s talk about how to do self-review well.

ProTips for self-review

You may be used to reviewing other people’s code, but it’s going to be a bit different reviewing your own. You’ve already built up a mental state of how your code works, and won’t be reading it for the first time. You’ll have a bit of a blind spot that other reviewers won’t have, making it more difficult to see the flaws.

You can do something about that. Here are a few tricks we’ve learned.

• Review your code in your code review tool, not your editor.
  This will help put your brain in review mode, and take you away from your editing environment. This little context switch can be enough to help catch problems you wouldn’t have seen before.

• Wait to post the code for review.
  If there’s no rush to land the code, wait a day, work on something else, and then come back to it. You’ll have paged some of it out of your head by then. You might be surprised by what you notice.

• Re-learn, don’t remember, how your code works.

  Pretend you’ve never seen your code before. Read the code line-by-line, character-by-character. Don’t make any assumptions. You may catch some confusing variable names, documentation, or bad logic this way.

• Use checklists for code review.

  Keep a checklist of problems to look out for. Your team’s style preferences, possible edge cases, possible memory leaks, exception handling… Anything you can think of that may apply to your changes, or that of your team’s. This will help take a load off your sleep-deprived brain.

• Read documentation, comments, and displayed text out loud.

  You should read through every error message, every piece of documentation, and every comment out loud. See if it makes as much sense as you thought when you wrote it. If it helps, try explaining it to a rubber duck.

• Take the opportunity to write more documentation.
  

  If your code is low on documentation/comments, this is a good time to write some. Go into detail, cover corner cases, or anything that may be valuable to another reviewer.

  By being forced to write about your own code as you read it, you’ll not only help ensure your code does what you expect, but you’ll help other reviewers get through a review cycle much faster.

  This also works pretty well for unit tests.

These are a few of our tips we use when reviewing our own changes. What are yours?

Read More

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