Slaying Dragon Code Reviews

Slaying Dragon Code Reviews

About the author

Adam Beaton is a senior software engineer at PullRequest. Prior to joining the team, he was a Software Development Engineer II at Amazon, senior software engineer at Mastercard, and senior software engineer at Applied Predictive Technologies.


images/how-to-review-big-pull-requests.jpg

In a perfect world, all code reviews would be small. We’d wake up each morning, refreshed and rejuvenated, log in to our computer to find only one or two tiny changes. Each changeset so miniscule that, in a blink of an eye, each review is approved in quick succession. We’re all wrapped up with code reviews for the day, and so we can pat ourselves on the back before we go on and program away in bliss.

If we were to engage in code-review-anthropomorphism, we’d like our code reviews to resemble baby yoda – cute and small, but with a lot of force. That is to say, making a big impact.

But we all know this isn’t a perfect world. Not every code review will be so beautifully small. Some pull requests are sphinxes with complex riddles to be solved. Some code reviews are ghoulish, bedeviling goblins, deceiving us at every turn. And others are big, fat, scary dragons.

If you’ve been reviewing code for any length of time, you’ve surely stumbled into a dragon’s den. Maybe, just maybe, you’ve published your own dragon code review. Here are some of the signs:

  • The code change touches hundreds or thousands of lines of code (loc)
  • The changeset changes more than one responsibility
  • No one wants to review it

Dragons are easily born, and there are a lot of great articles about ways to avoid or prevent them.

Instead I want to focus on how to slay dragons after they’ve been born. To mix metaphors, it’s no use crying over spilled milk. How can we more effectively close out large code reviews? How do we get motivated to review them? And why should you care?

Dedicate time

I once worked with a prolific code reviewer. In addition to being a team lead, this individual wrote complex back-end APIs, reviewed technical proposals, and was a code review maven. I remember receiving countless high-quality code reviews from this individual on numerous occasions, even when one or two of my PRs started breathing fire.

This team lead had a most peculiar habit for a programmer; a habit I believe to be a superpower. Instead of staying up late and arriving to work around 9:00 or 10:00, this person was up as early as 4:30 or 5:00am. He had four or five uninterrupted, productive hours to review code, work on his own code, and catch up on emails.

One challenge when tasked with reviewing a large code review is simply making time for it. In most software development roles, we spin a lot of plates. We bounce between meetings, writing code, reviewing code, writing proposals, planning the roadmap, answering questions – it’s dizzying. How effective do you think you would be if you had 5 uninterrupted hours of work each and every day?

While waking up that early might be out of the question, carving out time for focus is not. I’ve worked on multiple teams that created two “no-meeting days'' per week for heads down work. Similarly, I’ve had colleagues work with management to block off their calendar for similar periods of focus.

As dragon code reviews emerge, consider blocking off a few hours specifically for reviewing that changeset. That way, you know that you won’t be interrupted by distractions.

Ideally, there are pockets in your day aligned with this strategy. You might get into the office just a couple of hours before your team’s standup one morning, harnessing your enhanced focus. Or maybe you can schedule it right after lunch or your daily walk, when you are satiated and relaxed.

Slaying dragons takes time.

Commit to a deadline

One great way to force yourself to complete almost anything, including code reviews, is to commit to a deadline.

At standup or some other recurring meeting, simply commit. “I will have that pull request reviewed by Thursday.” It’s as simple as that.

Nothing lights fire under your booty like risking your professional reputation!

The dragon is doing pushups

The first stage of grief is denial. But ignoring that dragon of a code review is not helping it go away. If anything, it might get worse.

Let’s say, for example, your team follows an agile process like scrum. The longer you wait to review the PR, the longer it’s been since you were given any context as to its purpose. You may have to go back and review user stories or design documents to refresh your memory. Also, as some of your team members begin to ask questions and add comments, the size of the code review actually increases! Instead of reading each line in sequence, you’ll find some reviews pocked with comments, responses, and discussions.

Furthermore, the longer it takes for team members to review a code change, the more difficult it becomes to merge back into the source code. As the rest of the team continues to commit functionality and features, more and more merge conflicts arise. That leads to nasty merge commits and impromptu refactors from the code author that drag the code review out longer.

It’s important to remind yourself that, in the long run, It’s better to start reviewing dragon code reviews sooner rather than later.

Verify it’s a dragon

All too often, a review that seems scary at first turns out to be harmless. I can’t tell you how many times I’ve seen a code review where the majority of the lines of code come from a single package-lock.json. We’ve also seen pull requests where the majority of the changes come from translated text or uploaded image assets.

Sometimes, it pays to be dubious of your tooling. Just because there are thousands of lines in a code review doesn’t mean you’re necessarily on the hook to review thousands of lines of code.

Divide and conquer

In Greek mythology, one of Hercules Twelve Labours was to kill the Hydra. While a Hydra might not technically be a dragon, Wikipedia describes it as “a serpentine water monster,” which sounds close enough to a dragon to me.

If you’re unfamiliar with the Hydra, it basically grows back two heads every time you decapitate it. But, Hercules learned that by cauterizing or burning the necks, the heads wouldn’t grow back.

One suggestion for tackling a dragon code review is to chop off one head at a time. That is, break down the code review into its component parts and review each one separately. For example, if the code change includes a new back-end API and a new front-end component, review the back-end API first, then move on to the front-end component.

Depending on the size and modularity of the code in question, simply breaking the review down into chunks might make it manageable.

Stall for time

Dragons, in mythology, often have virtually impenetrable armor except for one weak spot. (SPOILER ALERT) In The Hobbit, for example, Smaug is slain by an arrow through his soft underbelly.

While you might not be able to vanquish a dragon code review with one brilliant comment, you may be able to hold it at bay. I recommend reviewers, if trying to buy some time, put off reviewing the code review for functionality. Instead, start by scrutinizing the code change for comments related to code style, syntax, best practices, and bugs. Addressing surface-level comments takes time and needs to be done regardless. And on large pull requests, it’s fair for code authors to expect multiple iterations.

By beginning the feedback process, you’ve also begun the process of understanding the changeset. You might, for example, be ready to ask intelligent questions about the various components. “How does component X relate to component Y?” Or, “Which directory should I review first, X or Y?”

In fact, you may want to ask these questions in the code review itself! Instead of you doing all of the heavy lifting, ask the code author to give you the necessary context. Put the ball back in their court.

Then, over time, you can begin reviewing the code at the macro-level. You can start reviewing each component and its unit test coverage, then begin to understand each component’s dependencies and relationships, and then move on toward reviewing the system holistically.

Review in person

Yes, I’ve authored dragon code reviews. I’ll admit it. While I strive to keep my code reviews small, I’ve made mistakes. To err is to be human.

One lesson I learned as a code author who has put out a massive code review is that it pays to provide context. Once when I wasn’t getting feedback, I decided to conduct an experiment. My theory was that the size of the changeset was intimidating. My peers didn’t know where to start. Would my peers agree to an in person code review?

I scheduled a meeting with two team members, picked at random, and asked if they would attend. While their attendance wasn’t mandatory, it would really help me out. To my surprise, both teammates agreed to participate.

We spent about 45 minutes together, walking through the code change. I gave background context, introduced the code architecture, and highlighted important files and concepts. Those 45 minutes together was far more productive than having them wade through the new feature on their own. After not having any comments on my pull request for several days, I started receiving feedback the next day.

If your coworker authors a behemoth code review, ask them if they’d be willing to go over the code with you in person. Maybe the two of you can simply start pair programming. If more than one approver is needed on the team, it might be worthwhile scheduling a meeting or reviewing the code over lunch.

Establish a fair code review process

Particularly if large code changes are being submitted to your codebase regularly or you seem to be the only person reviewing code changes, it may be time to suggest a more equitable code review process.

First, vocalize your concerns with your team at a shared meeting. This might be the time and place to remind the team about code review best practices, like putting out small code changes.

Second, work with your manager to see if there can be a rotation established when a code change reaches a certain threshold of lines of code. Track how often code changes exceed that number of lines of code and who ends up reviewing. Having metrics will help you convince your team members and manager that it might be time for a change.

If, over time, a pattern emerges, voice your concerns with your team or manager. Ultimately, it shouldn’t be up to just a single team member to face all of the dragons alone.

Augment your code review tool

As a member of the PullRequest team, I’d be remiss if I didn’t encourage you to consider investing in Code Review as a Service. We help teams conduct quality code review through expert engineers backed by best-in-class automation and static analysis.

Unlike the traditional model of only having team members within a development team conduct peer code review, PullRequest connects your team with our proven reviewers. Our reviewers come from companies like Microsoft, Google, Amazon, and Uber, and we only accept the top 1% of applicants. When a dragon rears its ugly head, reviewers in our network can help lighten the load.

Plus, our team tracks metrics and benchmarks on Code Review Size and Code Review Lifecycle Duration, helping you quantify the carnage of dragon code reviews.

Barter

If all else fails, go full Machiavellian. Turn this dragon code review to your advantage.

Perhaps you could commit to reviewing that huge code review if that pesky bug-fix was taken off your hands. Or maybe…

“He who fights too long against dragons becomes a dragon himself”

The fact of the matter is, it’s hard to review large code reviews. It’s challenging, effortful work. Effective code review takes a lot of time.

As such, there aren’t that many experienced developers capable (or willing) to take dragon code reviews head on. Reviewing large PRs thoroughly sets you apart. It helps show your manager that you are a leader, willing to do the dirty work. You’re a team player, a mentor, and code-quality champion.

Nietzche said “He who fights too long against dragons becomes a dragon himself…” Nietzche was probably using the negative connotation for dragon, a symbol of chaos and evil. That is, one who fights evil too long becomes evil himself.

I prefer to read this another way. If you fight dragons long enough, you too can become a dragon - a vaunted dragon code reviewer.


At PullRequest, we agree with the research. Small code reviews beat large code reviews any day.

But every blue moon, a big, fat, scary, dragon code review will appear. We hope these tips help you slay them.


About PullRequest

HackerOne PullRequest is a platform for code review, built for teams of all sizes. We have a network of expert engineers enhanced by AI, to help you ship secure code, faster.

Learn more about PullRequest

Adam Beaton headshot
by Adam Beaton

December 29, 2020