Tuesday, January 25, 2011

Code Reviews

As an engineer coming from Google where every line of code is reviewed by another engineer before it is submitted, I had assumed that there would be a publicly available open source tool to help us facilitate code reviews in a similar fashion outside of the Googleplex. As it turns out, there are very few available and almost all are for doing code reviews post-commit. This includes the code review tool built into Google Code Hosting. Google Code recommends that you submit to a branch and then request a code review of the branch before merging into trunk. While that method certainly works, it's cumbersome and the overhead is too high. What inevitably ends up happening is nobody ends up requesting code reviews and the whole process gets dropped.

In my experience, the only way to make sure code reviews actually happen is to enforce it in your tools. But that's a post for another day. We don't enforce code reviews here at Adku because we don't feel that code reviews should be mandatory just yet for a three person startup, but you can bet that once we get bigger, we will.

Anyway, after hours of searching and experimenting with various tools, we settled on Rietveld which was developed by Guido van Rossum, author of Python and Mondrian. The problem with Rietveld, however, is that it doesn't work out of the box! It is meant to be run on appengine, but only supports Django 1.1 so you have to change your appengine setup to use Django 1.1 and this is not a trivial process. Moreover, if you run Rietveld on appengine, most of your changes will be available publicly with no way to restrict access to your internal team without diving in and modifying Rietveld source code.

In the end, after a bit of troubleshooting, we successfully got Rietveld up and running and even set it up internally to restrict access.

Now that it's up, it is beautiful, works great, and is exactly what we were looking for. Typical use looks something like this
  1. make some local edits
    $ vi my_source_code.py
  2. send the changes to carlos for code review
    $ python rietveld/upload.py --send_mail -r carlos
  3. carlos gets an email that looks like this
    From:
    Date: Thu, Nov 4, 2010 at 11:19 AM
    Subject: better demo (issue24)
    To: carlos@adku.com


    Reviewers: carlos,

    Please review this at http://rietveld:8000/24/

    Affected files:
    ...
    ...
  4. carlos clicks on the link and sees the unsubmitted changes in a web ui with change highlighting, syntax highlighting, and an inline commenting ui
  5. carlos enters a few comments inline with the changes and hits send
  6. i make the changes suggested and re-send the code
  7. he reviews again and hits approve
  8. i submit the pending change in my local client

Monday, January 3, 2011

Peer reviews in a 4 month old, 3 person startup?

Even though Adku is only 4 months old, part of our company culture involves doing periodic NxN peer reviews. Everyone reviews everyone else and provides hard, candid feedback on strengths and weaknesses. But are peer reviews necessary for a 3 person startup that's only 4 months old you ask? Yes, it's critical, and I'll tell you why.

Company culture is very hard to change

Company culture is one of those things you need to engrain as early as you can. What you do in the beginning months of your startup will permeate through it's entire life and it's very hard to change. That's why we decided to have regular retrospective meetings to improve how we work, that's why we work on puzzles together every week, and that's why we take trips together and drink together.

Conflict resolution is critical to company success

The most common reason for company dissolution in the early stages is some kind of personal conflict. If the early team can not discuss issues like product direction options or annoying work habits openly and resolve them, then they are doomed to fail. I've seen this time and again. Peer reviews help make sure issues get raised, discussed, and resolved.

Deep insight into team strengths will help shape team responsibilities

Typically, engineering teams inside companies have managers. This is because managers add value. Don't get me wrong, there are certainly managers out there who decrease value, but in general, they exist because they can assign responsibilities intelligently and make adjustments based on team dynamics and individual strengths. Without a manager, the founding team has to do this collaboratively. Peer reviews are essential to make this happen.

Each individual's personal growth creates shareholder value

The output you create at your company today may be worth $500,000/year. If peer reviews end up making you more productive resulting in an increased output worth $600,000/year, then you've just increased the value of the company. To be fair, chances are you will also get a raise, but it won't be equal to your increase in output. This is because you yourself do not actually produce $600,000/year. You are leveraged by the rest of the company. An engineer at Google may output $10,000,000/year, but may only output $100,000/year at Microsoft for equivalent work.