Code Review in the Workflow

For the last six months or so, we’ve been working with eSpark Learning. eSpark is an education tech company that is focused on using iPads and the wealth of available iPad apps to customize the learning experience of students.

They’re a great company that’s been making some big moves recently. When their new CTO, Luke Shepard, came on board he asked us how we felt about adding code review to our development process.

I had mixed feelings at first. While I had talked to a few people who had great things to say about using code review in an Agile world, I couldn’t get my corporate code review experience out of my mind. I still have scars from the argumentative and unproductive meetings held at the end of projects. There were many complaints and issues raised, but no time or budget to fix the problems.

Needless to say, I was a bit nervous about the idea. After pushing those worries to the back of my head, we agreed to try code review. A few days later, Luke had set up Phabricator and we were up and running.

The basic flow was:

  • Create a feature branch locally
  • Write code (including tests) to implement the feature commiting the code as you go.
  • When the feature is done, run arc diff which will prompt for a summary of changes, a test plan and a few reviewers and will then create a code review request.
  • Everybody looks for code reviews that need to be done a few times a day, usually in the morning and at lunch. When you find code needing review, you look it over for style, correctness, test coverage and general architecture.
  • Based upon the code, you can either make requests for changes or approve the code as is.
  • If code is approved, the developer can run arc land which will then merge the code to master and push it out.
  • If the code needs revisions, you make changes and re-submit.

At first, the process was a little slow. It took us a week or so to get used to the flow of doing code reviews and figuring out how and when to provide feedback. After about a month, it felt like we were really getting the hang of code review. Now, 4 months later, I can’t imagine working without doing code reviews.

While there is a tiny bit of pain added by the review process, the benefits more then make up for it. The folks who work on Phabricator came up with a list of benefits of code review. We can vouch for their list. We have found that code review really does push us to make smaller, easier to understand changes. Most of our little problems like forgetting to add a file, misspellings, or simple mistakes are caught in review.

Using code review you quickly learn that large scale changes can’t be properly reviewed. This forces you to do a better job of breaking big changes into smaller, more modular changes that can be easily explained and tested. The end result is more small, incremental changes which are both easier to review and easier to code.

As a distributed team, performing code review also helps us stay up to date on what other members of the team are working on. While before, I would basically know what Audrey or Michael were doing, I now spend time looking at the details of their code. That helps me understand their architectural decisions as they make them, and helps us ensure that we have a cohesive architecture even if we’re all working on different parts of the project.

In short, code review is a huge win and we’re really thankful that Luke introduced us to it.