At Smooch, it’s common for interns to provide feedback on a senior developer’s pull request. Is that weird? Maybe, but then again, maybe not. Let me explain.
I’ve worked at large and small companies during my career and participated in a variety of code review cultures. On some teams, code review is a battleground where engineers wield seniority like a sword. On other teams code review is a clerical approval gate, where work languishes in a queue for days only to get a timid rubber stamp at the end without any feedback. On some teams, code review doesn’t happen at all — and a year later when you look at your colleague’s code for the first time it feels like the Lewis & Clark expedition.
But there are teams who take a more practical approach to code review —and they’re rewarded for it.
It’s natural to assume that because code review happens before deployment, its main purpose is to detect faults. A thorough code review might surface ideas for better internal design, but it’s ineffective when it comes to catching bugs. In fact, when a developer addresses a Pull Request nitpick in a rush to get something out the door they might end up introducing a bug at the last minute (we've all been there).
Modern software teams tend to ship code at a fast cadence. If you want to deploy new versions of your code weekly or daily, you need to handle fault detection with automated tests. Static code analysis performed by humans will never catch as many bugs as a comprehensive suite of automated tests.
Why review code?
I see three reasons.
These three reasons ought to be regarded as equal branches of the code review government.
Software design has the most obvious appeal. Code review occasionally does catch faults, and it's a great forum where non-functional aspects of software craftsmanship are discussed.
The other two reasons are often forgotten or neglected. For this reason, I want to focus on them specifically.
Learning the code
When you read a piece of code for the first time, it takes a little while to get through it. As you traverse the jungle, your mind hacks away the vines and places mental waypoints. These waypoints help you walk those trails with ease the next time around. The code landscape will evolve over time, and your waypoints will eventually grow stale. Performing regular code reviews is a great way to incrementally keep yourself up to date.
You also contribute new code to the project from time to time. Having an up-to-date understanding of the code allows you to do this more quickly. It reduces the chance of making incongruent design decisions and it helps to inspire worthwhile code refactorings down the road.
Learning how to code
This is by far the most under-appreciated benefit of code review. I’ve worked on teams where all code needed to be reviewed by a senior developer, or where only senior developers were expected to perform code reviews. Hogging the code review responsibility is a great way to stunt the growth of interns and junior developers.
Like any other trade, you can only learn how to create software by doing it.
Inspecting a code change, understanding its implications, and considering alternatives are skills that you can only learn through practice. If you're not reviewing code, you're not learning. Software is a young and evolving field. There are always new techniques and tools colleagues will want to show each other, which has nothing to do with seniority. We are all self-taught, it doesn't matter how long you've been in the business. I profess to be a senior member of my team and I learn new things from interns all the time. Code review is my primary channel for learning new sleight of hand.
Code review worst practices
While it's great to talk about what benefits good code review can bring it's equally important to talk about what can go wrong.
Suppose you were to select a small set of talented senior developers to serve as your "code review board," where all PRs must be approved by at least one of them. You've just implicitly painted this small team with shared accountability over everyone else's contributions to the codebase. They will be highly incentivized to nitpick. You've created an approval gate. Gross.
Worse still, this small team gets stretched thin tackling a code review workload that ought to be carried by the team as a whole. A "review board" PR process might ensure strong consistency in your code, but it will grind your schedule to a halt.
The blame game
It's important to talk about how a code review process can fall off the rails.
Have you ever shipped a bug? Of course you have. Whose fault was it really? You, the author of the code, or the developer who reviewed your code and approved your PR? It's a trick question. While you were worrying about who's to blame you should have been more concerned about accountability and autonomy. The difference is subtle but crucial.
Suppose we held reviewers accountable for all of the code they approve. The reviewer would be incentivized to request endless changes from the author, perhaps going back and forth over multiple revisions. They'll expect to have their way, even when the author disagrees.
At some point you begin to wonder if it would be simpler for the reviewer to just rewrite the code themselves. Then the epiphany hits you: perhaps the author is the one who should hold the accountability and ownership of the code they ship.
In the end, the code reviewer must serve as a guide, not a gate, and rank shouldn't matter. At Smooch, the author of the code has final say. Passionate disagreements are not uncommon. An author might disagree with a reviewer's PR comment, but they must act in good faith: they must at the very least explain their side before they proceed.
Hopefully by now it's clear why everyone on the dev team (including interns!) should be performing code reviews.
Culture of feedback
Have you ever noticed that software developers tend to be a bit pedantic? That's because compilers are pedantic and we talk to compilers all day every day. What you do all day tends to rub off on you in unexpected ways.
The same is true with code review. We spend hours doing code reviews. If you get accustomed to a nasty battleground PR culture, you'll be crusty and argumentative in real life. However, if you have a code review culture built on mutual respect and strong, clear accountability, where colleagues assume the best of intentions in every PR comment, you'll develop a team culture where feedback is openly given and graciously received.