Go to...

A Strategy for Code Reviews

I was coaching an organization the other day where they are trying to increase code quality. They believe they can accomplish this by increasing the frequency of code reviews. Currently, their solution is to get everyone into a room on a weekly basis, with junior developers demonstrating what they did and senior developers offering constructive feedback. If you’ve been reading my blog for a while now, you’ll know that I usually find structured lengthy meetings as a wasteful activity. I’m not saying there isn’t value in them. I’m just saying their cost is too much, relative to the value.

Weekly Code Review

Because the teams are trying to increase their delivery velocity, they are being forced to review their code delivery policies and rules. The weekly code reviews are creating too much of a delay. If a developer finished his or her feature on day one, they have a whole week to wait for the code review. If any changes are recommended, they lose an entire week because they still have to make the updates and get another review. The quality may be high but the velocity is low.

Pairing

My recommendation was the teams should begin pairing.  Upon review of their defined policies, it did not say they had to have a formal weekly code review meeting.  What it did say was no code would be merged into the trunk until another set of eyes had reviewed it.  By having a “code review” developer sit with the active developer, as he or she wrote code, it could be reviewed in real time and satisfy the policy.  The one week feedback loop would be shortened to an immediate feedback loop.

The Compromise

Paired programming can be a hard sell.  Management can find it hard to understand having two perfectly capable developers working on one piece of code.  But, if you look at the cost of waiting an entire week to get feedback and also having a group of developers stop work to attend a code review meeting, you can see there is a lot of wasted time and effort.  When I asked about this, I was told that the meeting was also an opportunity for developers to learn about other areas of the system.  My counterpoint was to rotate the pairs, to also offer a learning opportunity.

Because the teams would not agree to do formal pairing, but they certainly see the value in it, they decided to have a different (floating) code reviewer every day.  He or she will make themselves available the entire day to review code as needed.  I don’t completely agree with this approach but it’s certainly better then a weekly code review meeting.  They will be exposed to new areas of the system and the feedback loop will be shortened.  My hope is, in time, they will realize that having that second set of eyes sitting there full time will be the most efficient appraoch.

Image Quality by Pictofigo

About Derek Huether

I'm Vice President of ALM Platforms at LeadingAgile. Author of Zombie Project Management (available on Amazon). Novice angel investor.

2 Responses to “A Strategy for Code Reviews”

  1. James O'Sullivan
    March 7, 2012 at 7:49 am

    This is something I’ve struggled with over the last few years. I definitely agree that weekly code review meetings are definitely wasteful and the feedback loop is way too long and I agree that pairing is a great way to review code but I don’t believe that 100% pairing is the way to go. I enjoy pairing, but find it very draining so I like my pairing time as well as some individual creative time. The problem is then, when is the code review done for the latter?

    I’ve started to come to the conclusion that code reviews are wasteful activity full stop. I prefer to see everyone (junior and senior developers) pairing and mentoring that way. Yes there might be code that could be improved but with collective code ownership and pair mentoring things will get better over time.

    • derekhuether
      March 7, 2012 at 9:22 am

      I think people can learn a lot from what you just wrote.

      From my perspective, I see value in reviewing code, just as I see value in reviewing my blog posts before publishing them. Do my posts need a formal review by several editors a week from now? No. That sounds nuts. But as organizations grow, I see them being more risk averse and have false hope in adding “formal” activities as part of your development process. I do believe pairing would be the solution if code reviews are mandated. I see the more maturer organizations getting it.

Leave a Reply

Your email address will not be published. Required fields are marked *