If you are a tech company, and your people commit code, then you probably have some code review policy. And if you do not, you definitely should: you want to have an extra pair of eyes on the code that goes live. You certainly do not want a mistake to break things. And that is why you do pull requests to contribute to GitHub repos, and why Google employees must have a certain degree of maturity to commit code without review.
BUT, as long as that is a good idea, we must be careful to implement it the right way. Just enforcing reviews is not enough. You want to make the time between the code is sent for review and the code is deployed as short as possible. The longer the review time span is, the more work will be needed when the review comes. That is because:
- Who wrote the code simply does not have it fresh in their mind anymore. The context switch between the current task and the code he wrote days/weeks ago is just more demanding;
- Conflicts are more likely to arise, and then more work is needed in solving them;
- Other issues may depend on the code being held for review. Other people may spend (waste) time debugging an issue for which a solution is already available;
- If the repo is public, it makes more difficult for other to jump in and contribute, because they also have to be aware of all the pending code.
The right way to implement a code review system in a tech company
I think the best way to implement a code review system is to:
- Assign each code review to a particular member of the team. If everybody owns a task, then nobody does as well. That is why you want that particular review to be a responsibility of someone specific. An automated system can randomly assign a review to a team member.
- Each code review comes with a deadline. That’s it: code reviews are as important as any other task – basically because every other task often generates a code review at some point, so if we lag on those, nothing gets carried to the end and we are getting no work done at all! We may have different priorities associated with different deadlines, but we want each review to expire at some point (with the longest being a couple days)!
- Team members can turn down their assignments, but only if they have a good reason to. Again, if code does not get reviewed, it cannot go live, and thus the work has been done for nothing. Reviews are important and must be considered as such.
- Then just track how it goes: who is turning down most assignments? Is the weight uniformly distributed across the team?