A big chunk of the Fullstory engineering team formerly worked at Google, where there is a famously strong emphasis on code quality. Perhaps the most important foundational tenet at the big G is a practice called code reviews, or, more precisely, “pre-commit” code reviews. We continue the practice at Fullstory and hold it as sacrosanct.
Although much has been written about the benefits of code reviews, it isn’t yet a ubiquitous practice in the world of software development. But it should be, particularly for large engineering teams or teams with a flat management structure, e.g. no project managers or supervisors.
Contained herein are both the big, obvious engineering reasons you should adopt code reviews, as well as the more nuanced — but equally important — benefits to your customers and your own company culture.
How do code reviews work at Fullstory?
While working on a new feature, Dave (for example) will cut a branch from the current version of our master product and work exclusively on that branch, a process with which I’m sure most of the coding world is intimately familiar. But before he can reintegrate the changes into master, Pratik or another qualified engineer must review his work and give him the stamp of approval:
LGTM (Looks Good To Me)
If Pratik has an issue with the way Dave has designed or coded the work, they’ll have a discussion (potentially with a long volley of back-and-forth reasoning) until they reach an agreement. Or, if Pratik has no issues, he can LGTM the work right away.
An ounce of prevention is worth a pound of cure
Most software engineers would vouch for the research showing that problems found early — during design or coding — can be fixed many times faster than when they’re found later in production. (I’ve copied the relevant data below, but you can read more about the impacts of inadequate testing in this tome from NIST [PDF], if you dare.) In layman’s terms, catching bugs earlier on makes them faster and easier to solve.
And that’s to say nothing of the ill will avoided by not exposing your customers to buggy features in the first place. Taking a few extra moments to perfect code before pushing it to production spares time spent fixing it later, but more importantly, it spares your customers from developing a negative perception of your product and your capabilities as a development team.
What kinds of problems do code reviews prevent? They sometimes catch bugs, yes, but there are mixed reports of how reliably that works. In fact, static analysis tools and unit tests are much better than reviews at ratcheting up and maintaining correctness in individual pieces of code over time. But the role of code reviews at Fullstory goes much deeper than that anyway.
5 intangible benefits of code reviews — a bionic cultural hammer
We’ve found that the most powerful benefits of code review are the subtlest to measure and describe. Code reviews have many important side effects — arguably, primary effects — that we discovered at Google and have continued to evolve at Fullstory. For us, the practice of code reviews is a “bionic cultural hammer.” It’s a “cultural hammer” because it’s a tool that strongly shapes the way we work, and it’s “bionic” because it is more powerfully self-perpetuating than any passive “coding standards policy” could ever be.
The following 5 intangibles may not be able to be plotted on a chart, but are no less, possibly more, important than simply catching bugs:
1. Code reviews promote openness
Code reviews set the tone for the entire company that everything we do should be open to scrutiny from others, and that such scrutiny should be a welcome part of your workflow rather than viewed as threatening. It’s natural to feel threatened by critique, so you have to hold it as a sacrosanct practice to maintain the right mindset.
No discipline at Fullstory is free from critique. Our designers, as we’ve discussed, regularly have their progress checked by other designers and non-designers alike. The marketing and hugging teams review each other’s work before publishing (a practice which can be referred to as “flavor policing” if our storytellers produce something especially bizarre). Everything at Fullstory, even the dogma to which we subscribe, is open to being challenged by anyone, to maintain agility, freshness, and the openness necessary to learn and grow.
2. Code reviews raise team standards
Aside from promoting psychic flexibility, code reviews also help prevent the broken window effect by making sure all of our engineers share a similarly high set of standards. The mere act of saying explicitly to ourselves that we have very high standards perpetuates a virtuous cycle. People begin to want to honor our high bar of quality for its own sake, not merely to “get past the reviewer.”
Upholding the culture of code reviews becomes a mantle each of us proudly and voluntarily advances. Putting a well-crafted piece of code in front of your reviewer in the first place becomes more important than earning an eventual LGTM. Showing yourself to be “the most receptive” to even the most deflating critique is a badge of honor. Perfecting your tone as a reviewer so that you can deliver even harsh feedback in a respectful and palatable manner is a celebrated skill.
3. Code reviews propel teamwork
The act of reconciling different viewpoints between reviewer and reviewee can sometimes be a challenge. Opinions are often subjective (e.g. “I just don’t like how you’ve done this,” to which one might respond, “Yeah, well, I do like how I’ve done this”), and the reviewer might be making a good judgment call… or might just be obstructionist without quite realizing it. (Humans are complicated, amirite?)
So, how do people resolve those kinds of divergent perspectives? Becoming good at this activity is the essence of functioning effectively as an egalitarian team. You shouldn’t need or want a manager acting as an “adult” to come in and break a stalemate; you should want everyone in your company to have the conflict resolution skills to work through such situations rationally. Code reviews are neverending practice of your dispute resolution skills, which usefully spill over into many other areas of work life.
4. Code reviews keep security top-of-mind
How might code reviews affect the security of your application? Even if we assume that automated tools (e.g. vulnerability scanning) find individual security problems more reliably than code reviews, there’s much more to the story. Let’s talk at the “meta” level. What message does it send to every engineer that, upon every single change, their code will be reviewed for security problems?
First, it keeps security top-of-mind as code is being designed and written. The high team standards mentioned earlier make every engineer want to not only pass a code review, but genuinely impress the reviewer with their code’s quality, making it far less likely that security considerations would be forgotten or overlooked. It’s very embarrassing to fail to sanitize data and invite cross-site scripting vulnerabilities, for example.
Second, maintaining great security means keeping track of myriad evolving technicalities, and code reviews can be an effective way for engineers to train each other continuously and organically. If Jaime learns about a new exploit on Hacker News this morning, he can look for it in the code reviews he does this afternoon.
Contrast this with sending engineers to a security training seminar. It has some value, sure. But it sounds better on paper than it is in reality, because:
when you learn things out of context in a classroom, you don’t learn them nearly as well as in a contextual, hands-on scenario that you care about: your own code, for example, and
one-time training, to the extent that your team manages to retain anything they learn, becomes stale very, very quickly in the technology world.
Code reviews, instead, require both reviewers and reviewees to constantly update their security knowledge and to practice it in the most relevant context possible: inside the implementation of the product itself. Security is thus never treated as “a thing apart,” and security implications are a fundamental aspect of every change we make.
5. Code reviews frame social recognition
One surprising downside of an egalitarian team culture is that you don’t really have a boss to pat you on the back and publicize your accomplishments. Here, again, code reviews are a bionic cultural hammer.
Only qualified reviewers can LGTM someone’s branch; no one may begin their journey at this advanced position of trust. You have to schedule a Prove It (an internal name for meetings where the host must defend or prove their thesis of some sort) to convince existing reviewers to agree that you’re ready to become a trusted reviewer yourself.
This is no popularity contest; we use clear criteria as a backstop, and evidence of knowledge and skill are hard requirements. Becoming a code reviewer thus becomes a flattering rite of passage, the very existence of which reiterates to every person at your company that your processes reflect your emphasis on quality, security, performance, and every other value you wish to advance.
We shape our culture because it shapes us
Despite plenty of software tools that can automatically find many bugs and security problems in our code, why do we choose to spend so much time reviewing every ... single ... change? And what does any of this have to do with Fullstory’s mission to help teams deliver a perfect online customer experience?
We believe that our adherence to code reviews helps us rehearse our cultural priorities:
Have high standards (especially with respect to security),
Be open to being wrong on any topic, and
Remember to contextualize our individual goals in terms of how it affects our teammates, company, and customers.
These tenets feel like the cultural ingredients of quality code, happier colleagues, and a better experience for our customers.
But do high standards around code result in better products?
You might be wondering if all this effort around reviewing code pays off when it counts: building better software. It's a valid question.
Well, our customers tell us how much they love Fullstory, and we see that as a strong signal of success. But even so, applying high standards to our code won't prevent all bugs. So when unexpected software errors inevitably do make their way into our product, we turn to Fullstory and use session replay as a debugging tool.
(So if you like quality products and want to build better software, you might want to check Fullstory session replay out.)
If you end up putting code reviews to work—or use some similar process—we'd like to hear about it in the comments below.
