A couple of weeks ago at Unite Boston, I had the pleasure of giving a talk entitled How We Make Unity: A Look into How Development in Unity’s R&D Organization Works. If you missed it and would like to view the slides, they’re available online over here.
Unity’s development mainline is managed with a gatekeeper workflow. A gatekeeper workflow means that developers do not push their own changes to the development mainline, but instead a gatekeeper manages an incoming queue of pull requests, verifying that the pull request has gone through a few different quality control measures (mainly automated testing, peer code review, and risk assessment by the release manager), and merges them.
That’s all well and good when described at a high level, but I’d like to elaborate on this topic of peer code reviews. It’s definitely the item on the checklist I got the most questions about after the talk. Code reviews are such a fundamental component to development at Unity that I’d even go so far as to say they are a key element to making development work in the crazy, fast-paced, distributed world we operate in.
Why do you do code reviews?
Code reviews at Unity accomplish a few different things:
- Improve Code Quality – They’re an investment in the quality of our code and our product. Code reviews improve code quality by finding bugs before they hit mainline and cause problems for other developers or our users.
- Apply Expert Knowledge – Unity’s development culture is centered around empowerment and autonomy; any engineer can change any part of the code he wants. Code reviews are a way to apply the knowledge of domain experts and area owners, regardless of who makes the changes. They’re also a great way to help on-board and provide feedback to new developers.
- Help Domain Experts Retain Ownership – This is related to the previous point. Making sure a domain expert reviews all of the code that changes in his domain ensures that he’s able to retain ownership of his area, despite the fact that anyone can change the code in it.
- Knowledge Transfer – Finally, code reviews are a form of knowledge transfer. Unity’s engineering team is widely distributed across 4 continents around the world. It’s really hard to keep track of all of the changes everyone is making, but code reviews are form of knowledge transfer to other developers about what is going on in the codebase. They’re a way to help prevent knowledge silos and the “critical path developer” scenario.
What about the fact that it takes time for developers to review code?
Yes, the time developers spend reviewing code is time that’s spent away from writing code. And it’s definitely true that code reviews (especially in a codebase with a lot of churn), can end up taking a very large percentage of a developer’s time — particularly if the developer is very senior and has a large amount of domain knowledge and experience.
But, despite all this, I think it’s worth it. Developing Software is About More Than Writing Code. I strongly believe that the worst way to try to measure productivity or value is by measuring the amount of code a developer writes. In particular, the time spent reviewing code is an investment in your codebase and in your product. Think about the amount of time it takes to review a change and find a bug. Think about the amount of time it would cost a developer to have to reproduce, debug, fix, commit, and verify that bug if it had gotten into mainline. If it had been shipped, think about the cost to your customers.
You can also ease the pain by putting some steps in place:
- Make sure you have some redundancy in domain expertise within your team. If you don’t have this in the beginning, code reviews can be a way to help get it 😉 But you should always make sure there’s more than one person who can review an area of code.
- Experiment to find tools that work for you. Different development teams will of course have very different use cases. If you have the luxury of being a fully co-located team, the bulk of your code may be done as walk-throughs sitting at the same machine together. If you’re distributed like Unity is, the bulk of the reviews might be done through some web-based tool. Figure out what exactly you want the code review workflow to be like in your team and experiment to find something that works.
- Learn some patience. Accept that code reviews do slow the development process down a bit. Developers need time to get in the zone and focus on their own work. They cannot be productive in a purely interrupt-driven environment. My observation is that most developers at Unity isolate the time they spend reviewing code to 1 or 2 blocks of time per day.
So there you have it. We do code reviews — and you should to!