Disclaimer: This guide is for post Product-Market-Fit companies. If your company is pre-PMF, you should be optimizing for speed of iteration above anything else. If you’re pre-PMF your headcount should also be as small as possible, therefore a lot of the guidelines here don’t apply. Adapt according to your needs.
We’ve all seen something like this happen before.
We don’t blame you. There’s always pressure from management to ship stuff as quickly as possible, to ultimately allow the product to generate more revenue or traction. In fact that’s our end goal as developers.
However, having a culture of precarious or null code reviews leads to the accrual of technical debt.
What is a code review?
A code review is the process of vetting code by another developer. A developer sends new code to be integrated to the main branch, and asks a teammate to review it.
The main goal of code reviews is to maintain a high code quality on the codebase. A second programmer can possibly identify issues in the logic, the readability, the modularity, the dependencies or the functionality of a programmer’s code.
Having a well established code review process also helps educate your team. It helps developers learn from each other.
So what’s the best way to make code review work?
Make everyone do code reviews
Even the most junior developer should be reviewing the technical lead’s pull requests.
You might think this is counterintuitive. Wouldn’t this allow for a lot of pitfalls? Yes, but that actually leads to a benefit. A developer with a minor seniority will not necessarily understand everything about the code context, might not anticipate all the potential failures, and generally speaking will not catch as many errors as a senior developer.
This however, forces the senior developer to explain his code changes better.
There are 2 techniques to distribute how pull request reviews are distributed. GitHub, for example, provides the option to automatically assign a PR reviewer using both.
Round-robin focuses on distributing the number of reviews equally, regardless of the number of outstanding pull requests they have.
The load balancing algorithm focuses on distributing pull request reviews based on the number of outstanding pull requests each team member has. This algorithm tries to ensure that everyone reviews the same amount of pull requests in any 30 day period.
You can take a look at GitHub’s documentation to learn how to set this up for your team.
Both methods are completely valid, and which one to choose from depends on the specific characteristics of your team. We suggest round-robin if the seniority level of your devs is homogenous, and load balance if it’s heterogeneous.
Check for readability
Code must be readable to be maintainable. Code must be maintainable to be useful for a software development team. Here’s a checklist for code readability.
Does the pull request include useful passive documentation?
If you want to take a look at a pull request template, take a look at the one we have in our open-source repository.
How strict this should be depends 100% on the stage of the organization that owns the codebase. The testability requirements for a corporate, that has a well established revenue generating product, with millions of users; are different from the ones of an early stage startup that has the existential need of finding Product-Market-Fit, else they’ll run out of money. The former needs to optimize for strictness, while the latter needs to optimize for speed of iteration.
When you want to optimize for strictness, here’s what needs to be tested.
Check for potential security failures