---
title: "The Art of Code Review: Building Better Teams"
url: https://sanjayshankar.me/the-art-of-code-review-building-better-teams/
description: "Master code review with this engineering leader's playbook: what to flag, how to give feedback, async vs sync, tooling, and a 9-point review checklist."
author: Sanjay Shankar
date_published: 2025-11-28T18:29:05+00:00
date_modified: 2026-09-11T05:37:33+00:00
categories: [Leadership]
reading_time_minutes: 7
---

# The Art of Code Review: Building Better Teams

> How to conduct code reviews that build better teams and ship better software.

****Quick answer****

A great code review balances speed and depth. Approve fast, block rarely, focus comments on logic, security, and architecture (not style – automate that), and frame every suggestion as a question rather than a verdict. The best teams treat reviews as a teaching surface, not a gatekeeping checkpoint.

Code review is the single highest-leverage engineering practice I have seen across every team I have shipped with. When done well, it compounds knowledge, raises the floor for junior engineers, and shortens incident response times. Done badly, it becomes a velocity tax that no one wants to pay.

This is the expanded playbook I now use to coach engineering teams – what to review, how to give feedback that actually lands, when to block, when to approve, and the tooling that removes the busywork.

## Why Code Reviews Matter (Beyond Catching Bugs)

Most teams treat code review as defect detection. That is the smallest part of the value. The real return on a healthy review culture comes from four compounding effects:

1. **Knowledge distribution** – every PR is a teaching artifact. Reviewers learn the system; authors learn from reviewer questions.
2. **Consistency at scale** – code that ships through review converges on shared patterns, even without a written style guide.
3. **Junior engineer velocity** – fast, kind reviews are the single largest accelerant for engineers in their first two years.
4. **Shared accountability** – once a senior engineer approves a PR, the team owns the outcome. Production becomes a team artifact, not a single author’s risk.

A 2024 [SmartBear *State of Code Review* report](https://smartbear.com/state-of-software-quality/code-review/) found that 90% of engineers believe peer review improves overall code quality, and teams that review every PR ship with 36% fewer post-release defects on average. That is real money – and real on-call sleep.

## What to Actually Look for in a Code Review

After reviewing thousands of PRs, I review against a fixed mental checklist. The order matters: I always work from highest impact to lowest.

![Code Review](https://sanjayshankar.me/wp-content/uploads/2025/11/code-review-best-practices-1024x683.webp)

### 1. Correctness and Logic

- Does the code do what the PR description says it does?
- Are edge cases handled (empty inputs, nulls, concurrent writes, timeouts)?
- Are there off-by-one or boundary errors in loops and ranges?

### 2. Security

- Is user input validated and escaped before it touches a query, shell, or template?
- Are secrets pulled from the environment or vault, never committed?
- Are auth checks present on every endpoint that touches user data?

### 3. Performance and Scalability

- Are there N+1 queries hiding in a list view?
- Are caches invalidated correctly?
- Will this work at 100x the current load, or only at today’s scale?

### 4. Architecture and Boundaries

- Does this change respect module boundaries?
- Is the shared state added that creates new coupling?
- Is the abstraction the smallest one that solves the problem, or a speculative one?

### 5. Tests

- Are tests added for the new behavior, including the failure paths?
- Do the tests fail when the code is wrong (mutation test the change in your head)?
- Are tests deterministic, or will they flake on CI?

### 6. Readability

- Will a future engineer understand this in six months without context?
- Are names accurate?
- Are comments explaining *why* (non-obvious decisions) rather than *what*?

Style, formatting, import order, and lint rules are **not** on this list. Those belong to automated tools like Prettier, Black, ESLint, or your language’s standard formatter. If you are reviewing whitespace, your team is wasting human attention.

## Common Code Smells to Flag

These are the patterns I flag almost every week:

- **Silent error handling** – `except: pass` or empty catch blocks that swallow signal.
- **Speculative abstraction** – interfaces with one implementation and no second use case in sight.
- **God functions** – methods over ~50 lines doing three different jobs.
- **Magic numbers and unexplained constants** – `if user.score > 7` with no comment on why 7.
- **Untested feature flags** – flag added with no off-state test.
- **Mutable shared state** – module-level lists or dicts being written to from multiple call sites.
- **Inconsistent error contracts** – some functions throw, others return null, others return a tuple.

Flagging these consistently is what builds a team’s shared taste.

## How to Give Code Review Feedback That Actually Lands

The single biggest unlock for review culture is learning to phrase feedback so the author *wants* to address it. Three rules I coach every reviewer on:

### Rule 1 – Frame Suggestions as Questions

Instead of *“This is wrong”*, try *“Have you considered using X pattern here? It might help with Y.”* The question form invites a conversation; the verdict form invites defensiveness.

### Rule 2 – Separate Mandatory from Optional

Use explicit prefixes:

- `nit:` – small, optional, non-blocking.
- `suggestion:` – worth considering, but author’s call.
- `question:` – I want to learn or confirm something.
- `blocking:` – must be addressed before merge.

Most teams I work with adopt this convention within a sprint, and merge times drop noticeably the week after.

### Rule 3 – Praise the Good Stuff

If someone writes a clean abstraction, an elegant test, or a thoughtful migration, *say so* in the review. Public, specific praise is what makes a reviewer’s harder feedback feel like coaching, not criticism.

## Async vs Synchronous Code Reviews

Most reviews should be asynchronous. They give the author time to think, allow distributed teams to participate, and create a written record.

Move to a synchronous review (a 15-minute call or a screenshare) when:

- The PR is large and architectural (> ~500 lines of meaningful change).
- A comment thread has gone three or more rounds without convergence.
- The author and reviewer disagree on a fundamental approach.
- The change touches an unfamiliar area for one of the two parties.

In my experience, one short synchronous review can resolve what would have taken three days of async ping-pong – but defaulting to sync wastes everyone’s calendar.

## How to Handle Code Review Disagreements

Disagreements are a signal, not a failure. Three patterns I use:

1. **Cost the disagreement** – is this worth 30 more minutes of debate? If the cost of either approach is small and reversible, the author wins.
2. **Pull in a third reviewer** – when two senior engineers deadlock, a third opinion almost always breaks the tie cleanly.
3. **Decide and document** – once a decision is made, the author writes a one-line comment in the code or PR explaining the chosen tradeoff. The next reader thanks you.

The failure mode to avoid: a reviewer who blocks indefinitely without proposing a path forward. Block + propose is fine. Block alone is a process bug.

## Code Review Tooling I Recommend

- **GitHub Pull Request templates** – encode the PR description checklist (description, screenshots, test plan, rollback) so authors don’t have to remember.
- **CODEOWNERS files** – auto-route reviews to the right people; cuts review-assignment latency.
- **Reviewable.io** – for repos with very large PRs and multi-round threads.
- **Graphite** or **Sapling** – for stacked PRs in trunk-based development.
- **Conventional Comments** – the `nit/suggestion/blocking` prefixes mentioned above; spec at [conventionalcomments.org](https://conventionalcomments.org).
- **Danger.js / Danger Ruby** – automate the boring “did you update the changelog?” checks so reviewers never have to.

The rule of thumb: every minute you can move from human review to automation buys you a minute of attention on logic, security, and architecture.

## A Practical Code Review Checklist (Copy This)

Keep this as a saved reply in your review tool:

```
[ ] PR description matches the diff
[ ] Edge cases and error paths handled
[ ] No new auth or input-validation gaps
[ ] No N+1 queries or obvious perf cliffs
[ ] Tests cover the change and the failure paths
[ ] No new public API without docs
[ ] No magic numbers without comments
[ ] Names accurate; future-reader friendly
[ ] Feature flag has both states tested
```

If all nine pass, approve. If one fails, comment with `blocking:` and a proposed fix.

## Frequently Asked Questions

**How long should a code review take?**

**Aim for under 24 hours per round.** Research from Cisco’s classic SmartBear study found that review effectiveness drops sharply after 60 minutes per session and after 400 lines of code reviewed at a stretch. Break large PRs into smaller, reviewable units.

**How big should a pull request be?**

Under 400 lines of meaningful diff whenever possible. Reviewers’ defect-detection rate falls off a cliff above that threshold. Stack PRs or feature-flag partial work to stay under it.

**Should junior engineers review senior engineers' code?**

**Yes – this is one of the highest-leverage growth practices on a team.** Juniors learn the system fast, and seniors get a fresh-eyed read. The senior should treat junior questions as gold; the junior should ask without fear of looking dumb.

**What is the difference between a nit and a blocking comment?**

A **nit** is optional, stylistic, or trivially small – author can ignore. A **blocking** comment marks something the author must address before merge: a correctness bug, a security gap, a missing test for new behavior, or a violation of a team contract.

**How do you handle a reviewer who is too harsh?**

Coach them privately, not publicly. Share the conventional-comments prefix system, model the question-form rewrite, and pair them with a calmer reviewer for a week. Most harsh reviewers are not unkind – they have just never been taught that tone is part of the review.

**Should code reviews be mandatory for every PR?**

**Yes, with one exception:** trivially mechanical changes like dependency bumps, formatter-only diffs, or auto-generated migration files can be self-reviewed and merged. Everything else benefits from a second pair of eyes.

## The Bottom Line

The best engineering teams I have worked with treat code review as the **slowest channel through which culture moves**. Every comment teaches; every approval signals trust; every block, paired with a fix, raises the floor.

Building a culture of constructive code review takes time. It is also one of the highest-return investments you will ever make in a team – better than any tool, framework, or process you can buy.