Software code reviews, and other empty rituals that waste time
Time has a monetary value. Don't waste it guessing about future problems that are unlikely to occur.
Sometimes I write about leadership in a general sense, other times I write specifically about leading a tech team. This essay is an example of the latter, but I hope that every entrepreneur might appreciate the lesson here, which is that there are many rituals that are popular with some disciplines and professions, yet those rituals lack real value, and so if you’re an entrepreneur who is trying to move fast and save money, you’ll want to limit those rituals.
Nearly all software teams engage in code reviews, meaning that if one engineer writes some code, then other engineers can read that code and post criticism (typically using a site such as Github, which allows engineers to post code and review the code submitted by others). Sometimes engineers will leave dozens, or even hundreds, of comments, critical of the code that someone else has submitted. And the original engineer, who wrote the code, is expected to address all of the raised concerns before that code is allowed to go to production.
Over the years, I have become critical of code reviews. They burn up a lot of time and energy — which would be fine if they produced some important result, but they do not. While many of the comments made in code reviews might be interesting, they are not so interesting that they pay for themselves. If you put some dollar value on the time invested, you'll find that the vast majority of this process is simply burning money. And not only money, but also, as this article says, morale.
A curious fact about the politics of modern tech teams is that some of the same people who consider themselves "anti-bureaucracy" are strongly in favor of this type of bureaucracy, even when it brings no measurable benefits.
My opinion of code reviews has become more negative over time, mostly due to the fact that I have not seen it achieve reliably positive outcomes.
I'll give specific examples:
FutureStay had the worst code that I'd ever seen. When I joined the company I was horrified at the level of tech debt. And yet, for years, they had had a rule that at least two engineers had to review every PR (you can think of a PR as a request to push code to production). So this terrible code, the worst I'd seen in my 24 years of coding, had been approved by two engineers.
And likewise:
Open Road Media also had very bad code, and also had a rule that nothing could be pushed to production until at least 2 engineers had reviewed the PR.
In both companies the code reviews slowed down the deployments, slowed down the team, and created a culture of internal sniping, while leading to no improvements.
For the most part, if the engineers on a team are bad then the code reviews will also be bad, but if the engineers on the team are good, the the code reviews will not be needed.
But how can I say the code that I saw was objectively bad? Because many bugs were found in production. And what would have reduced the number of bugs in production? More automated tests, especially high-level end-to-end tests. And so I eventually came to this conclusion: it is best to skip code reviews, and instead have the team invest that same time and energy into writing more tests, especially high level tests.
As I've grown in my career, and taken on higher level management jobs, I've also realized that code reviews do not scale to large-scale leadership, but tests do. Code reviews seemed like a good idea when I was leading a team of 3 engineers, but not when I was leading a team of 30. When I was running a team of 30 I was not able to do code reviews myself, nor was I able to assess the quality of the code reviews being done by senior engineers. Therefore code reviews no longer worked for me as a tool for managing the team. When I was leading a team of 30, the only tool-of-oversight that worked for me was automated testing. And so I've concluded this is where the effort should be made. As we grow in our careers we need to be looking for tools that allow us to operate at a higher scale, and we should want to pull the whole company along with us, up to a higher level of productivity.
When I am running a team I will assign software developers the tasks of writing various tests, especially high-level end-to-end stress tests. These tests sometimes reveal problems. We then respond to the problems. But we don't waste time responding to imaginary problems that have not been proven by a test, which is to say we do not do code reviews.
If you ask an engineer to do a code review, you are asking them to brainstorm all the ways that things could go wrong. In fact, there are hundreds of ways that things can go wrong, but most of those things are unlikely.
You cannot waste time and energy trying to protect yourself from all of the many hypotheticals that can be imagined. Many of the concerns that will be raised are mere phantoms.
More so, if you ask your engineers to do code reviews, then you are turning all of them into managers. You are authorizing them to invent work for one another. They will look at someone else’s code, see something they don’t like, write a comment. And now the original engineer, who wrote the code, is not allowed to deploy that code to production until they’ve done the work necessary to respond to the issues raised by the other engineer. But do you really want to give your engineers the freedom to invent new tasks for each other? Who sets the priorities, you or them?
I've also come to realize that software developers, quite naturally, develop strong opinions about questions of style, and yet these questions have no long-term impact on the health of the tech in the company. Other than enforcing the rules of some automated linter, the time spent on issues of style are 100% wasted.
I have the impression that there is some middle era, in the career of a software developer, where concerns about issues of style tend to peak. The junior developer does not know enough to care about such issues, but somewhere between 4 years of experience and 10 years of experience, these issues feel important. I think you need more than 10 years of experience to see the waste. In particular, you need to run at least one large project where the team invests a lot of time into code reviews, and you need to notice how much tech debt builds up, despite the code reviews, to realize that code reviews do not offer a helpful path forward.
On recent projects I've told the team there will be no code reviews, but instead we will focus on building tests. I get a surprising amount of pushback on this. Less experienced software developers get angry with me and tell me that I'm being unprofessional. In some sense they are correct, in the sense that "professional" refers to "standard norms that are accepted by a profession" — I am deviating from those, clearly.
The most reasonable criticism I get is that code reviews could catch not just minor problems of style but also important problems with algorithms. What if, they ask, some junior developer introduces code that is ignorant of the implications of Big O Notation? What if they introduce code that runs in polynomial time? But I would ask, how do we know if it is running slowly? Big O Notation does not actually tell us if something is running slowly. We can only know that through tests. So let’s build tests that measure time, and stress test our system with heavy loads of traffic, to see if our system crumbles.
Big O Notation offers an excellent example of where software developers can worry about the wrong thing: what if a junior level software developer writes code that runs in polynomial time but on data that will only ever have a few hundred records? While the algorithm might be sloppy, the code will run quickly because there are few records. In that case, any time invested to find a better algorithm will be a poor investment. Once I'm running a team of 30, the only thing I care about is whether code is actually slow, not whether it could theoretically be slow.
But wait, what if the main problem, which would be discovered during a code review, is that a sloppy engineer wrote the code in a way that is difficult to test? Well, you discover that by writing the test. Can you write a test for that code? If not, then you finally have some real-world problem that you should respond to. But you don't try to predict that ahead of time.
Kent Beck, when he invented Xtreme Programming, also popularized the phrase "Do not write a comment in your code, instead, write a method with an easy-to-understand name that does what the comment was going to communicate."
I've come to a similar conclusion. Do not write a comment in a code review, instead, write a test that would catch whatever danger you want to warn about. And if your concern does not materialize when we run the tests, then the thing you were afraid of is probably not worth worrying about.
But wait, you might ask, are there not large-scale issues of architecture that are too large to test? Yes, but those large-scale issues are the kinds of issues I can focus on when I’m running a team of 30. Indeed, cutting out wasteful practices such as code reviews, which focus on pointless minutiae, frees up time we can then spend focused on the big issues which will actually determine the fate of the company.
I’ve focused here on software code reviews, but hopefully you can see this is one specific type empty ritual, whereas you should be worried about the general case. Will a given marketing campaign be misunderstood? Well, how can you test it on a small scale? Will your 3PL disappoint you with poor staffing at the warehouse? Well, how can you test that (you can order things from them before you sign a contract)? There are a hundred such cases. In all of them, you’ll want to avoid the avoiding of unproven worries. You’ll want to find ways to figure out what problems are real, and focus your energy on fixing those.
Asking the team to brainstorm all possible problems mostly generates a long list of time-killing tasks that lack any real-world value. You should try to find tests that will reveal the real problems, but aside from what you can prove to be a problem, you should focus on building something great. You don’t want to waste your time in a defensive crouch, you should want to go on offense and dazzle the world.