The Blog

Everyone knows code reviews can be awesome. They can be agile, smart and effective, or they can be shallow and pedantic. They can be an hour-long laser-focused meeting on the points that matter, or a rambling mess of code reading to sleepy co-workers desperate for some cookies and a nap. I’m on the team that builds backend services for the latest and greatest games; we take code reviews very seriously, and we look forward to them. So how do we go about it?

We’ve developed two schemes for official code reviews, and each one serves a unique purpose. The first approach grew organically and is informal, whereas the second is a relatively new scheme that we’ve adopted and has already been very successful.

Our older code reviews are somewhat similar to how many other teams conduct their reviews. A major new piece of code is written, and if on general consensus it is deemed “important”, each person on our team, or at least 2-3 of us, are responsible for giving it a thorough review, writing down their notes and attaching them to a Lighthouse ticket. We schedule a time for review, and then gather as a group and walk through each and every item of note, discussing as necessary. Usually the original developer leads the review, though one of the reviewers may also lead the discussion. We typically use this style of code review for new modules and services which are deployed in development or staging, to be entering production “in the future”. It is the first pass review, intended to bring many eyes to bear on immature code, where only one or two people may have been involved in implementation.

We’ve adopted a more formal practice for services and modules entering production. For any given title which is nearing production, we list all of the services it is using, including ones that may be very mature. For each service, we assign two reviewers and one change implementor, preferably none of the people who originally wrote the module, though in a small agile team this can be open to interpretation or convenience. The reviewers each perform a rigorous analysis, covering everything from code to configuration, interaction with other modules and services, algorithm efficiency, test suite coverage and whether the entire package conforms to the latest team practices. Our standards continue to evolve, and so even mature services benefit from this continual review process. A third person, the change implementor, then applies the recommended changes as well as anything else they see as needing improvement. The reviewers then review the changes, recommend additional fixes, pass back to the change implementor, and so on, until all 3 individuals agree that the service is ready for production. We make liberal use of git branches to track these changes. Near launch day, a fourth team member is responsible for confirming that the production deploy is operational; configurations comply with the final requirements, databases and other datastores are setup as designed, and AMQP hosts are operating as expected.

In addition to these two formal schemes, we also use git and Campfire to continually review smaller changes. Each team member discusses their recent work in our daily standups and brings attention to code they think needs attention from others, and everyone takes part in reviewing changelists and recommending improvements as their schedules allow.

We’ve realized great benefit of these two approaches in combination. The former yields an informal but wide review of the code base, giving everyone an opportunity to understand the bleeding edge of our technology. Sometimes this includes learning an entirely new language or framework during the course of the review. The latter style yields a production-ready deliverable that has been approved by no fewer than 4 members of our team. In practice, the entire team has had a chance to either code a feature, review it, or implement changes, such that everyone can sign off that we are ready stand behind the coming tsunami of happy gamers playing the best new titles on the market. Both schemes allow individuals to thoroughly review the code at their own pace and according to their own schedules, so that group time is dedicated solely to discussing the items of importance. Everyone is involved in the success, proudly putting their signature on the final product even when their role in supporting a particular title may have been minor.