← Back to Blog

Code Review That Actually Helps: Lessons from Both Sides

• 12 min read
code-review collaboration team best-practices software-engineering

Code Review That Actually Helps: Lessons from Both Sides

Early in my career, I left a code review comment that really upset a teammate.

I didn’t mean to. I thought I was being helpful. But reading it back now, I cringe:

“This entire approach is wrong. You should delete this and start over with a proper architecture.”

No explanation. No suggestion of what “proper architecture” meant. Just criticism.

I’d focused on the code and forgotten there was a person on the other side.

That was four years ago. Since then, I’ve been on both sides of hundreds of code reviews. I’ve learned what makes reviews productive, what makes them soul-crushing, and how to give feedback that actually helps people grow.

The Goal of Code Review

I used to think code review was about catching bugs and enforcing standards.

It’s not. Or rather, those are side effects, not the main purpose.

The real goals:

  1. Share knowledge across the team
  2. Catch issues before they reach production
  3. Improve code quality collaboratively
  4. Help developers grow

If your code review process isn’t achieving all four, something’s wrong.

What I Got Wrong as a Reviewer

Mistake 1: Reviewing the Code, Not the Change

I’d open a PR and start commenting on every issue I saw. Old code that wasn’t part of the change. Style preferences unrelated to the feature. Architecture decisions made months ago.

The developer couldn’t address most of my comments because they weren’t touching that code.

Now I do this: Review only what changed. If I spot issues in surrounding code, I create a separate issue or mention it in a 1:1. The PR stays focused.

Mistake 2: Being Vague

// My old review comments:
"This could be cleaner"
"Not sure about this approach"
"Consider refactoring"

What does “cleaner” mean? What’s wrong with the approach? Refactor how?

These comments create work without providing direction.

Now I write:

// Specific, actionable feedback:
"This function does three things: validation, transformation, and saving.
Consider splitting into validateInput(), transformData(), and saveToDb().
This would make each piece easier to test and modify independently."

Explain what’s wrong, suggest a solution, explain why it matters.

Mistake 3: Treating Everything as Blocking

I used to leave 30 comments, all implying the code couldn’t merge until addressed.

The developer would spend a day addressing minor style issues while an urgent feature waited.

Now I categorize comments:

  • Blocking: Security issues, bugs, broken functionality. Must fix.
  • Suggestion: Would improve the code but not required. Consider for this PR or future.
  • Nitpick: Style preferences, minor cleanups. Nice to have.
  • Question: Genuinely curious, not implying it’s wrong.

I prefix comments so intent is clear:

[Blocking] This SQL query is vulnerable to injection. Use parameterized queries.

[Suggestion] Consider extracting this into a helper function. Would make it reusable.

[Nitpick] Personally I'd name this `userCount` instead of `count`, but either works.

[Question] What's the reason for using Map here instead of an object?

Mistake 4: Forgetting to Say What’s Good

Humans remember negative feedback more than positive. If I leave 10 comments, all pointing out problems, the developer feels like they failed. Even if 95% of the code is great.

Now I intentionally call out good patterns:

Nice use of early returns here. Makes the logic much easier to follow.

Good catch on the edge case handling. I wouldn't have thought of that.

The test coverage on this is solid. Clear setup and assertions.

It costs me nothing and makes a real difference to morale.

What I Got Wrong as the Developer Being Reviewed

Mistake 1: Taking Feedback Personally

Someone would suggest a different approach, and I’d feel attacked. “What’s wrong with MY approach? Do they think I’m stupid?”

I’d get defensive. Argue in comments. Make excuses.

What I learned: Feedback on code is not feedback on me as a person. The reviewer is trying to improve the codebase, not attack my competence.

Now when I receive feedback, I assume positive intent. Even if the comment seems harsh, I ask myself: “Is there something useful here?”

Mistake 2: Submitting Huge PRs

I’d work on a feature for two weeks, then submit a 1,500-line PR.

Reviewers would glaze over. They’d miss important issues because there was too much to process. Reviews took days.

Now I split PRs:

  • Infrastructure/setup: One PR
  • Core logic: One PR
  • UI integration: One PR
  • Tests: Included with each PR, not separate

Each PR is reviewable in 15-20 minutes. I get faster feedback, and reviewers catch more issues.

Mistake 3: Not Providing Context

I’d submit a PR with a one-line description: “Add user profile feature”

Reviewers had to figure out what problem I was solving, what approach I took, and why. They wasted time understanding before they could review.

Now I include:

## Summary
Add ability for users to edit their profile photo and bio.

## Problem
Users had to contact support to update their profile.
20+ support tickets per week for this.

## Solution
- Added profile edit form at /settings/profile
- Used existing image upload component
- Added validation for bio length (max 500 chars)

## Testing
- Manual testing on Chrome, Firefox, Safari
- Added unit tests for validation logic
- Screenshots of mobile/desktop views below

## Questions for Reviewer
- Should we add image size limits? Currently accepts any size.
- Is the 500 char limit reasonable for bio?

Good context makes reviews faster and more useful.

Mistake 4: Ignoring Review Comments

I’d get a review with 15 comments, address 10, and click merge.

The other 5? “Minor stuff, doesn’t matter.”

This eroded trust. Reviewers felt ignored. They stopped putting effort into reviews.

Now I address every comment. If I disagree, I explain why. If I’m not fixing something, I acknowledge it:

Good point, but I'd rather address this in a follow-up PR
to keep this one focused. Created issue #234 to track it.

Every comment deserves a response, even if the response is “noted for later.”

Patterns That Work

The 24-Hour Rule

Reviews sit for a maximum of 24 hours. Longer than that, PRs pile up, developers context-switch, and everything slows down.

I block 30 minutes each morning for reviews. It’s part of my job, not extra work.

Pair Review for Complex Changes

For architectural changes or tricky logic, I’ll jump on a call with the author instead of writing 50 comments.

“Can you walk me through this?” often resolves confusion in 10 minutes that would take an hour of async comments.

Automated Checks Before Human Review

Humans shouldn’t review formatting. Machines should.

# In CI pipeline
- run: npm run lint
- run: npm run type-check
- run: npm test

If these fail, the PR doesn’t need human review yet. Fix the obvious stuff first.

Review Your Own PR First

Before requesting review, I look at my own diff. Fresh eyes catch obvious issues: console.log statements, TODO comments, unused imports.

This respect for the reviewer’s time makes them more invested in giving good feedback.

How I Handle Disagreements

Sometimes I’m reviewing code and I genuinely disagree with the approach. Not a bug, just a different preference.

What I used to do: “This is wrong, do it my way.”

What I do now:

  1. Ask first: “What led you to this approach?” Maybe they have context I don’t.

  2. Share my perspective, not mandates: “In my experience, X tends to cause problems because Y. What do you think?”

  3. Accept their decision: If they’ve heard my concern and still prefer their approach, I let it go. Unless it’s a clear bug or security issue, healthy teams allow different styles.

  4. Escalate rarely: If we’re truly stuck, we involve a third person. But this should be rare, not the norm.

The goal is consensus, not winning arguments.

The Review Checklist I Actually Use

When I’m reviewing, I go through this mentally:

Functionality

  • Does the code do what the PR description says?
  • Are there obvious bugs or edge cases?
  • Do the tests actually test the right things?

Security

  • Any user input going directly to SQL/HTML? (Injection risks)
  • Sensitive data being logged or exposed?
  • Authentication/authorization correct?

Maintainability

  • Will someone understand this in 6 months?
  • Are there magic numbers or unclear variable names?
  • Is complex logic explained with comments?

Performance

  • Any obvious N+1 queries or expensive operations in loops?
  • Large data sets handled correctly?
  • Unnecessary work being done?

Integration

  • Will this break anything that depends on it?
  • Are database migrations safe?
  • Any deployment considerations?

I don’t comment on everything. Just what matters.

The Human Side

Code review is collaboration between humans. A few things that help:

Assume competence. The author isn’t stupid. If something looks wrong, there might be context you’re missing.

Be kind. “This doesn’t work” vs “I think this might break in scenario X. What do you think?” Same content, different tone.

Remember the goal. We’re trying to ship good code, not prove who’s smarter.

Say thank you. When someone gives you a thorough review, thank them. It takes time and effort.

What Changed for My Team

After improving our code review culture:

  • PR turnaround dropped from 2-3 days to under 24 hours
  • Fewer bugs escaped to production
  • Junior developers ramped up faster (reviews became teaching moments)
  • Less friction between team members

The code didn’t change that much. The conversations around the code changed everything.

If You Take One Thing Away

Code review is a conversation, not an inspection.

Your job as a reviewer is to help make the code better while respecting the person who wrote it.

Your job as an author is to make the reviewer’s job easy and take feedback as a chance to learn.

Do both well, and code review becomes one of the best parts of team software development.