Code reviews that actually work

Code review is one of those practices that nearly every team claims to do, yet few do well. All too often, reviews are superficial passes that catch a style nit or two, offer little substantive feedback, and result in merges that leave everyone feeling uneasy. The problem isn't that teams skip reviews—it's that their review practices don't produce the learning, knowledge sharing, and quality bar they're meant to deliver.

At Shopify, we treat code review as a core engineering skill. Our values—shipping fast, learning continuously, and building for the long term—sometimes pull in different directions. These practical techniques are how we reconcile them and get reviews that genuinely improve both the code and the people writing it.

The anatomy of a bad review

Consider a familiar scenario: A developer spends three weeks on a task, submits a pull request with roughly 1,000 lines of new code, gets a couple of style comments and a question from a reviewer who clearly lacks context on the work, receives approval after minor fixes, and merges with fingers crossed. Then comes months of anxiety, waiting to see when and how the code breaks.

The red flags here aren't hard to spot. The PR is far too large for anyone to review meaningfully. The reviewers didn't have the context needed to evaluate whether the solution was even correct. And the author walked away with no confidence in their own work. That's a process failure, not a personal one.

The fixes below address those failures directly.

Keep pull requests small

The single highest-impact change you can make is to keep pull requests small. Two reasons drive this:

  • Reviewers are far more likely to actually start and finish a review when the diff is manageable. Large PRs invite procrastination and make it easy to get interrupted mid-review and never return.
  • Deep review is exponentially harder on a large diff. Building the mental model needed to evaluate hundreds of lines of changed code across multiple files is a heavy cognitive load, and reviewers naturally skim instead of examine.

Breaking work into smaller units doesn't just make reviewing easier—it tends to produce faster, more thorough feedback. There's no universal size limit that works across all languages and project types. At Shopify, our data engineering teams use roughly 200–300 lines of changed code as a guideline; above that, we almost always split the work.

That said, be careful not to split too aggressively. When PRs get very small, reviewers may need to see several of them to understand the full picture, and context gets lost in the fragmentation.

Use draft PRs to validate direction early

It's tempting to think you need a polished, complete implementation before showing anyone your work. That instinct produces the classic "building a car instead of drawing one" failure: you spend months building the wrong solution because you never checked the requirements with anyone before committing to an approach.

Software has the same trap. You can work for a long time on a feature or module that ultimately doesn't meet user needs, and the waste isn't just the weeks of effort—it's the polish, documentation, and detail you applied to a fundamentally wrong direction.

Shopify's answer is to solicit early feedback through WIP PRs. The goal at this stage isn't line-level review—it's validating the direction: the choice of algorithm, design, API, or architecture. Early feedback means less wasted effort on details that may become irrelevant.

This requires authors to hold their opinions loosely. We aim for strong opinions, loosely held—make decisions confidently, but stay open to changing course when given sufficient evidence. GitHub's draft PR feature is useful here because it signals that work is still in flux and prevents accidental merges. If your tool lacks that feature, a clear WIP label on a regular PR works almost as well.

One PR per concern

Line count isn't the only dimension that matters. A single PR can address multiple concerns—a feature plus a refactor, two bugs at once, a dependency upgrade bundled with a new service. Each of those should be its own unit of work.

Splitting PRs by concern has several concrete benefits:

  • Review units are more independent, so each review gets more focused attention.
  • Fewer domains of expertise are needed per review, meaning fewer people need to be pulled in to evaluate a single diff.
  • Rollbacks become atomic. When something breaks, you can identify what to revert without also undoing unrelated changes.
  • Easy changes get separated from difficult ones. If a new feature requires refactoring a widely used API, the bulk of the diff is obvious, functional-change-free updates to call sites, while the feature itself needs careful review of test coverage, error handling, and intended behavior. Bundled together, reviewers must skim everything to find the parts that matter. Split apart, the routine work lands quickly and review effort concentrates where it's needed.

Avoiding mixed concerns also speeds up iteration. When a PR spans multiple issues, feedback on one part can arrive days before feedback on another. The author waits, shifts focus to unrelated work, and loses momentum. Split into smaller units, the author can begin addressing feedback as soon as any single review completes, creating a pipeline of review and revision rather than a long stall.

Review the code, not the person

Review communication matters as much as the technical content. The goal is to improve the product, not to make the author feel personally judged. A few practices help keep reviews constructive:

  • Frame feedback as "this is our code; how can we improve it?" rather than "your code has problems."
  • Comment on what's done well. Positive remarks reinforce good practices and give the author a balanced picture of the feedback.
  • As an author, assume best intentions. Read comments as efforts to improve the work, not as attacks.

Tone and wording matter. A comment like "This logic is wrong" can easily be rephrased to focus on the design question rather than the author's competence. Similarly, questions such as "Why did you choose this approach?" invite conversation, while flat assertions shut it down.

A good review is a teaching opportunity, for both the reviewer and the author. Treat it as such.

Choose reviewers wisely

Picking the right reviewers is often harder than it seems. Useful questions to ask when deciding who should look at your work:

  • Who has context on the feature or component being changed?
  • Who is strongest in the language, framework, or tooling involved?
  • Who has strong opinions on this subject?
  • Who actually cares about the outcome of this work?
  • Who should learn this material? If you're a junior review someone senior's work, ask your questions—even the ones that feel basic. Strong teams make time for knowledge sharing.

Whatever your team's standing review rules may be, the author bears the responsibility of seeking out reviewers who have the right context to provide a genuinely useful review.

Map the territory for your reviewers

A good PR description can make or break a review. Different reviewers will have different levels of context on the work. The author's job is to close that gap by providing the key information—or linking to it—so reviewers can offer meaningful feedback.

A useful PR description answers several questions:

  • Why is this PR necessary?
  • Who benefits from it?
  • What could go wrong?
  • What alternative approaches were considered, and why was this one chosen?
  • What other systems does this change affect?

Good code isn't just bug-free—it's useful. The PR description should tie the code back to team objectives, ideally by linking to the related feature or bug ticket. Reviewers should start with the description before judging code quality; if the description is incomplete, send the PR back. Without defined objectives, no one can fairly evaluate whether the code achieves them.

Sometimes the best outcome of a review is realizing the code isn't needed at all. Keep that possibility on the table.

The broader payoff

Beyond faster shipping and cleaner code, these practices create cultural effects that compound over time:

  • Shared understanding. When the whole team reviews work, no single person is the only one who can evolve any given part of the codebase.
  • Shared responsibility. When something breaks, it's the team's code that needs fixing, not one individual's.

That shared context and ownership is what allows any one person to take a real vacation—multiple days, fully disconnected—without worrying that the world will end while they're away.

Getting started on your team

If you lead a team, treat these techniques as experiments. Try them, see what works in your context, and adjust.

If you're an individual contributor, talk to your lead about why code review practices matter for effectiveness and team growth. Bring it up at your next one-on-one or team sync. The conversation itself is a first step toward reviews that deliver on their promise.

Why Reviews Deserve Real Investment

When a team debates whether to invest time in code reviews, the argument often comes down to speed versus quality. The short-term view says shipping working code quickly matters most. The long-term view understands something else entirely: every review is a chance to make a teammate stronger, and a stronger contributor writes better code on every future task—not just the one under review.

A lead engineer framed it this way:

We could prioritize landing mediocre but working code in the short term, and we will write the same debt-ridden code forever, or we can prioritize making you a stronger contributor, and all of your future contributions will be better (and your career brighter).

An enlightened author should be delighted to have this attention.

That framing reframes the review process. It is not a hurdle or a formality. It is an investment that compounds. When the reviewer’s intent is to develop the author rather than just police the diff, the conversation changes. Authors stop defending their work and start mining it for insight. Reviewers stop listing nitpicks and start teaching principles the author can apply next time.

The benefit is not one-directional. Reviewers gain too, by reading unfamiliar code paths, catching their own assumptions, and seeing how other people solve problems. The whole team builds a shared vocabulary around what "good" looks like, which makes future discussions faster and less personal.

This is why the best teams protect review time instead of cutting it. They also treat the review as part of the work, not as an interruption or afterthought. A thoughtful review conversation is often where the team’s standards get written and rewritten, one diff at a time.