Code Review #

Code review is one of the activities done most often in engineering teams yet discussed least deeply. Many teams run it as a formality — the reviewer opens the diff, adds one or two comments, then clicks approve. The result: preventable bugs slip into production, code standards stay inconsistent, and engineers don’t grow because they never get meaningful feedback. This article covers code review as it should be — as one of the most efficient investments an engineering team can make for long-term quality.

What Is a Code Review? #

A code review is the process where code written by one engineer is reviewed by another engineer before merging into the main codebase. What’s reviewed isn’t just whether the code works — but also whether it’s easy to understand, consistent with team standards, secure, free of unnecessary performance problems, and maintainable by others in the future.

A good code review is a technical dialogue, not a one-sided audit. Reviewer and author both play active roles in producing better code than either could produce alone.

flowchart LR
    subgraph CodeReviewAsTechnicalDialogue
        A["Author\nwrites code"] -->|opens PR| B["Reviewer\nreads and understands"]
        B -->|constructive feedback| C["Discussion\nfinding the best solution together"]
        C -->|revisions| D["Better code\nthan either could produce alone"]
    end

Code Review Fundamentals #

Code Is a Shared Asset, Not an Individual’s Property #

Once code merges into main, it’s no longer the author’s. It’s a team asset that anyone on the team must be able to read, understand, and modify — including an engineer joining a year later. Code review is the mechanism ensuring this asset can truly be “shared understanding”.

// Implications of "code is a shared asset"
✓ Code should be written to be read, not just to run
✓ Naming, structure, and comments are forms of communication — not decoration
✓ Reviewers have the right and obligation to ensure code is understandable to them
✓ Authors can't reject feedback with "that's just how I code"

// What changes when a team truly holds this principle
From: "This is my code, please don't comment too much"
To:   "This is a PR for our code — I welcome comments to make it better"

Bug Costs Rise Drastically Over Time #

This is the easiest calculation for understanding code review’s value. The same bug has very different costs depending on where and when it’s found.

flowchart LR
    A["Bug found\nat code review"] -->|cost| B["Minutes to hours\nedit code, push again"]
    C["Bug found\nat QA/staging"] -->|cost| D["Days\nassign, fix, retest"]
    E["Bug found\nin production"] -->|cost| F["Days to weeks\nhotfix + incident + data impact"]

    B -.->|10x cheaper than| D
    D -.->|10x cheaper than| F

One code review comment preventing a production bug can save tens of hours of engineering time. That’s an ROI no other engineering investment can match.

Knowledge Sharing Happens Naturally #

Code review is the most effective form of knowledge transfer because it happens in a real context — not in a training room or presentation session, but directly on the code being worked on. Reviewers learn about the changes the author made. Authors learn from the reviewer’s perspective, which may be more familiar with a certain domain. Other engineers reading the discussion thread learn from both.

// Knowledge that moves during code review

From Author to Reviewer:
  → How the new system works
  → Design decisions made and why
  → How a certain problem was approached

From Reviewer to Author:
  → Better patterns or idioms in that language/framework
  → Missed edge cases
  → Implications of changes on system parts the author didn't touch

From the Discussion Thread to Everyone Reading:
  → Reasoning behind technical decisions
  → Trade-offs considered
  → Prevailing team standards

Consistency Over Personal Preference #

Code review isn’t a place to impose personal coding style. Its main goal is consistency — code written in the same style across the codebase is far easier to understand than code that’s “most elegant” per each individual.

// ✗ Comments imposing personal preferences

"I'd prefer this variable to be named X"

"I think this way is more elegant"

"At my previous workplace, we always used approach Y"

// ✓ Comments maintaining team consistency

"Other services use the X naming convention —
 can we stay consistent here?"

"Other code in this repository uses pattern Y for similar cases.
 Any particular reason to use a different approach here?"

The Real Value of Code Review for Teams #

Maintaining Long-Term Codebase Health #

Teams that don’t take code review seriously tend to accumulate technical debt invisibly — every small PR adds a little inconsistency, a small anti-pattern, or an unnecessary dependency. Over time, the codebase becomes increasingly hard to modify.

Without serious code review:
  Months 1-3: Code is still understandable
  Months 6-12: "Don't touch the code here, nobody understands it" appears often
  Year 2+: Major refactors needed but always postponed because they're too risky

With serious code review:
  Anti-patterns caught before entering the codebase
  Inconsistencies fixed before spreading
  Technical debt controlled and resolvable incrementally

Enforcing Standards Without Policing #

Code review is the most natural mechanism for enforcing engineering standards — without micromanagement or long rule documents nobody reads. Standards form organically from the discussions happening in PRs.

Standards organically enforced through code review:
  ✓ Coding conventions (naming, structure, idioms)
  ✓ Architecture (layer boundaries, dependency direction)
  ✓ Security (input validation, data sanitization, credential handling)
  ✓ Performance (N+1 queries, unnecessary computation, memory usage)
  ✓ Testing (coverage, test naming, test quality)

Accelerating Engineer Growth #

Junior engineers getting quality feedback from senior reviewers can grow faster than achievable through courses or books. Conversely, senior engineers learn from fresh perspectives often missed because they’re too familiar with the system.

flowchart TD
    CR[Code Review] --> JL["Junior → Senior faster\nDirect feedback in real context"]
    CR --> SL["Seniors keep growing\nFresh perspectives from junior engineers"]
    CR --> TL["Team collectively stronger\nKnowledge spread, not locked to individuals"]
    CR --> BF["Lower bus factor\nMore engineers familiar with every area"]

How to Read a PR Effectively #

Many reviewers open the diff directly and start reading line by line. That isn’t the most effective way. A better approach:

The optimal PR reading order:

  1. Read the PR title and description first (2-3 minutes)
     → Understand the PR's purpose before seeing a single line
     → If the description is missing or unclear, ask the author to complete it first

  2. Look at the changed file list (1 minute)
     → Get a picture of the change scope
     → Identify which files are most critical to review

  3. Read the added tests (if any) (3-5 minutes)
     → Tests explain *what* the code is supposed to do
     → This builds a mental model before reading the implementation

  4. Read the implementation — starting from the most critical (10-30 minutes)
     → Use the mental model from steps 1-3 as a guide
     → Ask: does this do what the description says?

  5. Verify test coverage (2-3 minutes)
     → Do the existing tests cover the newly implemented behavior?
     → Are the edge cases you found already covered?

Best Practices from the Author Side #

Self-Review Before Opening the PR #

Engineers who review their own code from a reviewer’s perspective will find most problems before other reviewers need to find them.

Questions for effective self-review:
  □ Is every changed line truly necessary?
  □ Are there variable or function names that would confuse others?
  □ Are there debug comments, console.log, or forgotten TODOs?
  □ Is there logic that could be simplified without losing clarity?
  □ Is this the easiest-to-understand code for achieving the same goal?

Explain Non-Obvious Decisions #

Reviewers don’t share the author’s context. Decisions that feel obvious to an author who spent hours with a problem are often not obvious to a reviewer seeing it for the first time.

// ✗ Code without context — reviewers guess why
time.Sleep(500 * time.Millisecond)

// ✓ Code with a comment explaining the decision
// The payment gateway requires a minimum of 500ms between requests to
// avoid rate limiting. See RFC-021 for full context.
time.Sleep(paymentGatewayRateLimit)

Respond to Feedback Openly #

Code review feedback isn’t a personal attack — it’s an additional perspective from someone who wants the team’s code to be better.

// ✗ Defensive responses closing the discussion

"It's already correct, I've thought about it"

"This is my preference, leave it alone"

// ✓ Responses opening technical discussion

"Thanks for the feedback — you're right about this edge case.
 I'll fix it by adding validation here."


"I chose this approach because [specific technical reason].
 Do you have specific concerns with this approach?"

// If you disagree with the feedback — still discuss with technical arguments

"I understand the concern, but I think this approach is still better
 because [reason]. Can [the reviewer's concern] be addressed with [solution]?"

Best Practices from the Reviewer Side #

Read the Description Before the Diff #

This is the single easiest but most impactful habit change for review quality. Reviewers who understand the PR’s purpose before reading the diff give far more relevant, on-target feedback.

Prioritize by Impact #

Not all code review comments carry equal weight. Reviewers who spend energy on minor things while missing critical problems aren’t giving meaningful reviews.

The correct priority order:

  [CRITICAL] Correctness — is the logic right?
    → Logic bugs, off-by-one errors, possible null pointers, race conditions
    → This is what MUST be found before code reaches production

  [HIGH] Security — are there vulnerabilities?
    → SQL injection, unvalidated input, hardcoded credentials, insecure random

  [HIGH] Architecture — does it match the agreed design?
    → Layer boundary violations, excessive coupling, violations of existing RFCs

  [MEDIUM] Performance — are there significant performance problems?
    → N+1 queries, unnecessary computation, potential memory leaks

  [LOW] Readability — is the code easy to understand?
    → Confusing names, overly long functions, unclear logic

  [INFORMATIONAL] Style — does it match coding conventions?
    → Lowest priority and best handled by automated linters

Give Context to Every Comment #

Comments without context don’t help authors prioritize or understand why a change is needed. Every meaningful comment must answer: what the problem is, why it’s a problem, and what can be done.

// ✗ Comments without context

"This is wrong"

"Change this"

"Not good"

// ✓ Comments with full context

"This loop will do N+1 database queries — one query for every
 order in the list. With 1000 orders, this produces 1001 queries
 and slows this endpoint down significantly.

 Consider using eager loading:
   db.Preload('Items').Find(&orders)
 
 This reduces it to 2 queries (1 for orders, 1 for all items)
 regardless of the number of orders."

Label Comments Clearly #

Without labels, authors don’t know which comments need immediate action and which are just suggestions. A simple labeling system removes this ambiguity.

// An effective labeling system

[BLOCKING] — Must be fixed before the PR can merge

"[BLOCKING] SQL injection vulnerability — user input goes directly into the query string"

[SUGGESTION] — Recommended but optional, not blocking

"[SUGGESTION] This function could be simplified using helper X
 which already exists in the utils package."

[NITPICK] — Minor, no impact on logic or maintainability

"[NITPICK] Typo in the comment: 'recieve' → 'receive'"

[QUESTION] — Needs clarification, not necessarily a change

"[QUESTION] Why is the timeout 30 seconds? Specific context for this number?"

[FYI] — Information that may be useful, no response needed

"[FYI] There's an RFC-018 discussing an alternative approach for cases like this,
 maybe useful as a reference."

Give Praise for the Good Things #

Code reviews containing only criticism create an environment that feels like an audit. Reviewers who also appreciate good solutions build a healthier collaborative relationship.

// ✓ Sincere, specific appreciation

"The retry approach with exponential backoff here is excellent —
 the jitter handling is also accounted for.
 This is more robust than the previous implementation."


"The separation of concerns here is very clean. Easy to test separately."

// ✗ Too-generic appreciation (almost useless)

"Good job!"

"Nice code!"

Review in a Timely Manner #

PRs left waiting too long for review harm everyone. Authors lose context because they’ve moved to other tasks. Merge conflicts accumulate. Delivery slows.

Review SLAs commonly adopted:
  Normal PR (< 300 lines): first review within 4 working hours
  Large PR (> 300 lines): first review within 1 working day
  Urgent PR/hotfix: review within 1 working hour

If you can't review now, a useful minimal response:
  "I'll review this PR tomorrow morning — I have another deadline today"
  "I skimmed it, there's one thing I want to ask first: [question]"

What must NOT be done:
  → Leaving a PR without any response for more than 2 working days
  → Approving just to "clear the queue" without reading

The Role of Automation in Code Review #

One of the most effective ways to improve human code review quality is moving automatable things to machines.

flowchart TD
    PR[PR opened] --> CI[Automatic CI Pipeline]
    CI --> L["Linter\nformatting, style, unused imports"]
    CI --> SA["Static Analysis\npotential bugs, complexity"]
    CI --> T["Test Suite\nunit, integration"]
    CI --> SC["Security Scanner\nvulnerability, credential leaks"]
    CI --> B["Build\ncompilation, dependency resolution"]

    L & SA & T & SC & B --> R{All passing?}
    R -- Yes --> CR["Human review starts\nfocused on logic, design, context"]
    R -- No --> A["Author fixes first\nbefore review starts"]

The right division of responsibility:

  Machines (automatic) handle:
  ✓ Formatting and indentation
  ✓ Unused imports and variables
  ✓ Agreed linting rules
  ✓ Test coverage thresholds
  ✓ Common security vulnerabilities (SAST)
  ✓ Dependency vulnerability scanning

  Humans (reviewers) focus on:
  ✓ Correctness of business logic
  ✓ Fit with architecture and design
  ✓ Edge cases not covered by tests
  ✓ Readability and maintainability
  ✓ Trade-offs not visible from code alone

// ✗ Reviewers wasting time on automatable things
Comment: "Line 42 indentation is inconsistent"
Comment: "There's an unused import on line 3"
→ These should be handled by linters, not human reviewers

// ✓ Reviewers focusing on added value
Comment: "[BLOCKING] Potential race condition here when two goroutines..."
Comment: "[SUGGESTION] This pattern could be simplified using..."

If the team doesn’t yet have a CI pipeline running linters and automated tests on every PR, that’s the first investment to make before optimizing the human code review process. Automation eliminates an entire class of unnecessary comments and lets reviewers focus on what truly needs human judgment.

Code Review Anti-Patterns to Avoid #

// ✗ LGTM Theater — approving without reading
Reviewer opens the PR, scrolls briefly, clicks Approve
→ The most dangerous false sense of security
→ Preventable bugs slip into production
// ✓ If you don't have time to review well, say when you can
   Don't approve just to "clear" notifications

// ✗ Perfectionism blocking delivery
Reviewer blocks a PR over style preference disagreements
Endless comments on non-critical things
→ PRs sit idle for weeks, delivery stalls
// ✓ Apply a threshold: if there are no blocking issues, approve
   Non-blocking issues can be followed up in the next PR

// ✗ Review as a power tool
Senior reviewers blocking junior PRs to show authority
Comments belittling the author's ability
→ Juniors stop taking initiative, a culture of fear forms
// ✓ Review is collaboration, not hierarchy. Even juniors can give
   valid feedback to seniors — what matters is the technical argument

// ✗ Scope creep in reviews
Reviewers start questioning large architecture decisions that aren't
this PR's scope and were decided long before the PR existed
→ The PR can't merge because the discussion scope is unlimited
// ✓ If there's a big architectural concern, open a separate discussion (RFC)
   Review the PR within its scope

// ✗ Passive-aggressive comments

"This code is clearly wrong"

"Did you even read the documentation?"
→ Authors get defensive, discussions become unproductive
// ✓ All feedback delivered respectfully and based on technical arguments

// ✗ Reviews focusing only on style, missing logic
10 comments on naming and formatting
0 comments on the logic bug in the main function
→ The PR looks reviewed while its critical problem goes undetected
// ✓ Prioritize correctness first, style later

Effective Code Review Checklist #

BEFORE STARTING THE REVIEW:
  □ Read the PR description and understand its purpose before opening the diff
  □ Confirm CI has passed — don't review PRs with red CI
  □ Estimate the time needed — allocate a sufficient time block

DURING THE REVIEW:
  □ Check correctness: is the business logic right?
  □ Check security: any unvalidated input? hardcoded credentials?
  □ Check architecture: does it match the agreed layer boundaries?
  □ Check performance: any N+1 queries? unnecessary loops?
  □ Check tests: do they cover new behavior? critical edge cases?
  □ Every comment includes context and why it matters
  □ Comments labeled clearly: [BLOCKING], [SUGGESTION], [NITPICK], [QUESTION]
  □ Appreciate the good things — not just criticize

ENDING THE REVIEW:
  □ Give a clear decision: Approve, Request Changes, or Comment
  □ If Request Changes: make sure all blocking items are clearly written
  □ If there's major ambiguity: consider a short sync with the author

AFTER AUTHOR REVISIONS:
  □ Re-review only the changed parts — no need to re-review the whole PR
  □ Verify all blocking comments are properly addressed
  □ If satisfied: Approve and make sure the PR is ready to merge

Summary #

• Code review is a technical dialogue, not an audit — reviewer and author both contribute to producing better code than either could produce alone.
• Bugs found at review are far cheaper — one comment preventing a production bug can save tens of hours of engineering time.
• Code is a team asset, not an individual’s property — once merged, everyone is responsible for its quality.
• Prioritize correctness, security, and architecture — style is the last priority and best handled by linters.
• Label comments clearly — distinguish blocking, suggestion, nitpick, and question so authors can prioritize responses correctly.
• Give context to every comment — “this is wrong” doesn’t help; explain why and offer an alternative.
• Good automation elevates human review quality — solid linters and CI free reviewers to focus on what needs human judgment.
• Timely review is a form of respect — PRs waiting too long waste the author’s context and hinder delivery.
• LGTM without reading is the most dangerous anti-pattern — it creates a false sense of security while letting problems into the codebase.
• Specific appreciation builds a healthy culture — genuine positive comments are as important as constructive critical ones.

← Previous: One PR, One Purpose   Next: Code Review Meeting →