Code Review Culture: Feedback That Actually Helps

Why Code Review Fails Teams

Code review is one of the highest-leverage activities in software development. Done well, it spreads knowledge, catches bugs, and improves design. Done poorly, it creates resentment, slows delivery, and teaches people nothing.

I've seen both. Here's what distinguishes the two.

The Four Types of Review Comments

Not all feedback is equal. Being explicit about the weight of a comment reduces friction.

nit: Minor style preference, author can ignore or fix
     "nit: I'd name this `userCount` instead of `count` for clarity"

suggestion: My recommendation, but I understand if you disagree
            "suggestion: Could we extract this into a separate function?"

question: Genuine curiosity, not a disguised criticism
          "question: Why do we need to fetch this on every render?"

blocker: Must be addressed before merging
         "blocker: This will panic on nil input — we need a guard here"

When you don't label your comments, the author has to guess whether you're blocking or nitpicking. That ambiguity breeds anxiety and slows down the review cycle.

What Reviewers Get Wrong

Reviewing Style, Not Substance

Style debates (tabs vs spaces, quote style, naming conventions) should be automated. If your linter and formatter don't catch it, add a rule. A human reviewer's time is too valuable to spend on formatting.

# These should never appear in a code review comment
# Configure once, enforce automatically
npx eslint --init
npx prettier --write .
npx tsc --noEmit

If you find yourself writing "use double quotes here" in a review, stop and add a Prettier rule instead.

Rewriting, Not Reviewing

The difference between feedback and a rewrite request:

// ❌ "Just rewrite it like this:"
// Reviewer pastes a 40-line refactor
 
// ✅ Explain the principle, offer the option
// "This function is doing three things — validation, transformation, and persistence.
// Could we split it into smaller functions? I'm happy to pair on this if helpful."

The author learns more from understanding the principle than from copying your solution.

Drive-By Reviews

A review that only catches syntax errors and ignores architectural concerns is incomplete. A review that obsesses over variable names and ignores a missing security check is dangerous.

The review hierarchy:

1. Correctness — does it do what it claims?
2. Security — does it handle untrusted input safely?
3. Performance — are there obvious inefficiencies at scale?
4. Design — is the abstraction appropriate?
5. Readability — can the next developer understand it?
6. Style — is it consistent with the codebase? (automate this)

Most style comments belong at level 6. Most PR discussions happen at level 6.

What Authors Get Wrong

Massive Pull Requests

A 3,000-line PR will get a shallow review. Reviewers lose motivation, skip sections, and approve out of exhaustion.

Target: under 400 lines changed per PR. For large features:

• Feature flags — merge incrementally behind a flag
• Stacked PRs — foundation → abstraction → feature
• Interface-first — define the contract, then implement

# ❌ One monolithic PR
[FEATURE] Add e-commerce checkout flow (3,247 lines changed)

# ✅ Decomposed into reviewable chunks
[1/4] Add cart data model and repository layer (280 lines)
[2/4] Add cart API endpoints with validation (310 lines)
[3/4] Add checkout UI components (420 lines)
[4/4] Wire checkout flow end-to-end (180 lines)

Each PR is mergeable independently and tells a complete story.

Poor PR Descriptions

A PR description is a cover letter for your code. It should answer:

• What does this change do?
• Why is this the right approach?
• What alternatives were considered?
• How can the reviewer test it?
• What should they look at first?

## What
 
Adds rate limiting to the public API endpoints to prevent abuse.
 
## Why
 
We've seen automated scraping causing load spikes on `/api/products`.
Limiting to 100 req/min per IP address matches industry norms for public APIs.
 
## Approach
 
Using Redis sliding window with `rate-limiter-flexible`.
Considered IP-based vs API-key-based — went with IP for now since we don't
have API keys yet, and can layer key-based later.
 
## Testing
 
- Run `npm run test:rate-limit` to see the rate limiting behavior
- Or hit the endpoint 110 times in rapid succession in dev
 
## Areas to focus on
 
- The Redis key structure (line 47) — I'm not sure it's optimal
- Error response format (line 89) — should match our existing error shape?

Building a Healthy Review Culture

Respond within 24 hours. Stale PRs are demoralizing and create merge conflicts. If you can't review today, say so.

Separate review from approval. You can leave comments without blocking the PR. Use "Request changes" only for actual blockers.

Celebrate good code. Reviews aren't only for finding problems.

// "Really elegant approach to the retry logic here — borrowing this pattern."
// "Nice catch on the edge case in the empty array handling."

Assume good faith. The author made reasonable decisions given their context. Questions first, conclusions second.

Keep the scope tight. If you notice unrelated problems while reviewing, create separate issues — don't expand the PR's scope.

The best code review I ever received taught me a new pattern, validated my approach, and left me energized to keep working. That's the bar.