Code Review on Paper

A day or so ago, news broke that engineers at Twitter - newly-owned by Phony Stark - were being asked to “print out their last 30 to 60 days of code, so they could show it to Elon Musk himself”. We saw evidence of this from Twitter employees themselves:

This is ridiculous for a number of reasons, which I’ll describe briefly. But it’s more interesting because of the reactions to it.

Wait - you don’t like Code Review?

First off, let me be clear. Code Review itself - the process of submitting prospective code changes for comment and approval before they are merged into the codebase - is unequivocally a good thing.

• Most obviously, it gets more perspectives on the code to catch potential issues or suggest improvements¹
• It subconsciously prompts you to work at a higher quality, because you know you’ll have to justify any shortcuts or corner-cutting during review.
• An often-overlooked advantage is that it spreads awareness of what other people are working on and what’s changing in the codebase, and gives you a chance to see techniques or approaches that other people are using. This is why I push back on the idea that code can only be someone of equal or higher level than the author - while a junior engineer is probably unlikely to notice issues with a senior’s code, they will still benefit from the experience of reading and understanding it!
  • This also acts a legibility filter - if there is insufficient description in the commit message to explain or justify the change, or if it uses unnecessarily² “clever” code, a junior is much more likely to notice that than a senior³. Remember that code is written only once, but might need to be read many times over the course of its lifetime, and you can’t guarantee that the person who wrote it will be the person who wrote it.

There are certainly issues that arise during the practice of Code Review, but in my experience they’re usually process problems rather than problems with the practice itself⁴. E.g. the response to “people aren’t doing Code Reviews, so we’re not able to ship any changes” shouldn’t be to remove Code Reviews, it should be to understand why people aren’t doing Code Reviews⁵!

OK, so why is this a bad idea?

Leaving aside the absolute nonsense of requiring a printout rather than using existing online Code Review tools⁶ - the idea of reviewing 50 pages of code with a single person is just mind-boggling. With an estimated 7,500 employees, and generously assuming that the Elongated Muskrat can read, understand, and discuss one page a minute (without breaks for weak human needs like food, sleep, or hygiene), and maintain focus throughout that time, that’s 260 days, or nearly 3/4 of a year, that would be spent solely on this process. Not exactly the best use of this hyper-genius’ time.

But wait - we said elsewhere that a big benefit of Code Review is in knowledge-sharing? Maybe the 1:1 bottleneck was miscommunicated, and this is in fact a way for incoming engineering leaders to get up-to-speed on the state of the codebase? Well, if that were the case, then focusing on the last 1-2 months of changes is a terrible idea. That will only show what’s changed over that time period, when new leaders would want to get high-level architectural overviews, descriptions of key components, processes, and user flows, and discussion of pain points and planned improvements - all of which can be accomplished without reading a single line of code. Trawling through recent commits alone won’t get you much useful insight; and if it does, it will be by spawning incidental tangential conversations ("Why did you have to change the timeout on the Frobulator? // Oh, so there’s this interesting component interaction we have to work around…").

The only reasonable interpretation is that this is not a means to understand the codebase, but a way to evaluate the authors in advance of making cuts (or even a tactic to get people to quit). This is also deeply flawed, because “reading the code you committed” is a terrible metric for impact⁷. Engineers - especially senior engineers, whose impact is most important to judge - have impact in many ways beyond merely typing code. Oncall, mentoring, researching, prototyping, and pair-programming are all activities that don’t result in commits, to say nothing of attending meetings⁸. Most importantly, writing and presenting documents is an integral part of having broad impact - “Principal Engineer is where you swap your IDE for Microsoft Word”. Judging engineers on their recent code contributions is an effective way to insult the high-performers, leaving only the less-impactful employees to keep the company running. I’m sure this will go just fine.

Why are the reactions interesting?

I found some reactions to the tweet fascinating. Note that there’s no context or sentiment provided in the original tweet, so readers are free to interpret it however they wish. Many - as I did - laughingly shrugged at how Kafka-esque the whole process is. Some, however, felt the need to defend the practice, asserting that engineers should be willing and able to demonstrate their value to the company, and that anyone who was unwilling or unable to do so was probably dead weight.

This is an interesting perspective, because it focuses solely on the personal experience of a policy without considering its broader implications. If someone asked me to print off my changes and describe their value in order to justify my employment, I wouldn’t be offended because it was belittling - I’d be offended because it was a wasteful and inefficient way to accomplish the stated goal. By assuming that the original tweeter is offended at the insult rather than the inefficiency, these Musk-defenders are projecting that people only oppose policies that harm them personally and that prevent them from fitting in or being rewarded, rather than opposing policies that are seen to be harmful or counter-productive even when one can benefit from them.

I wonder if there might be a correlation with one’s personal politics?

• politics
• real-life