Code Review Culture

Code Review Culture

Code review is the most common form of knowledge transfer and quality assurance in software teams. Done well, it improves code quality, spreads domain knowledge, mentors junior engineers, and builds team consensus around design principles. Done poorly, it creates friction, slows delivery, and damages relationships.

The Purpose of Code Review

Code review serves multiple goals, not all equally important:

Primary: catching bugs, security issues, and architectural problems that the author missed.

Secondary: ensuring consistency with codebase conventions, sharing knowledge about changed code with at least one other engineer, and maintaining a shared understanding of how the system evolves.

Tertiary: style enforcement, formatting — these should be automated (linters, formatters) and never the subject of review comments.

The purpose defines the standard: a review that finds no bugs isn't a failure — it's validation. The goal is not to find something to criticize.

Reviewing Well

Understand before critiquing: read the PR description, the linked ticket, and understand the intent before reading the diff. A change that looks wrong often makes sense given context you don't yet have.

Review the right things: logic correctness, security implications, error handling, test coverage, and alignment with architecture. Not variable names, brace style, or line length — those are for formatters.

Be specific and actionable: "This looks wrong" is not useful. "This will panic on nil input if user.Profile is null — add a nil check" is. The author needs to know what to fix and why.

Distinguish blocking from non-blocking: label critical issues clearly. Label suggestions as optional. The author should never have to guess what they must fix versus what they can consider.

Conventional Comments labels:

• blocker: — must be resolved before merge
• suggestion: — optional improvement
• question: — seeking understanding, not necessarily requiring change
• nit: — minor preference, not blocking
• praise: — positive signal, encourages good patterns

Acknowledge good work: reviewing is not purely adversarial. When you see elegant code, an insightful test, or a well-documented complex function, say so. Praise reinforces good patterns and builds psychological safety.

The Author's Responsibilities

Write a useful PR description: context saves reviewers time. What problem does this solve? Why this approach and not alternatives? What edge cases did you consider? What's in scope and what's left for later?

Keep PRs small: the correlation between PR size and bugs found per line reviewed is inverse — reviewers lose focus on large diffs. PRs under 400 lines receive 90% review effectiveness; PRs over 1000 lines are often rubber-stamped.

Self-review before submitting: read your own diff. You will catch 20-30% of issues yourself.

Respond to all comments: even to say "acknowledged, will address in a follow-up" or "disagree because X." Leaving comments without response creates confusion about what was addressed.

Don't take it personally: a comment on your code is not a comment on your worth as an engineer. The best engineers welcome critical review because it improves the code.

Giving Feedback on Sensitive Issues

When reviewing code by a senior engineer or addressing a fundamental design issue, frame feedback as questions: "I'm wondering if we should consider X — what are your thoughts on the tradeoffs?" rather than directives.

When disagreeing with a reviewer, provide reasoning: "I considered that approach but chose this one because of performance implications in the hot path — see the benchmark in the PR description." Respectful technical disagreement is healthy.

When the same issue recurs in multiple PRs, address it in a team discussion or a code style guide rather than relitigating it in every review.

LGTM Culture and Review Quality

Teams under delivery pressure often develop LGTM culture — approvals without substantive review. This is a false economy: bugs that pass review cost 10-100x more to fix after deployment than during review.

Signs of LGTM culture: approval within 5 minutes of a large PR, zero comments on complex changes, authors who never receive blocking feedback.

Addressing it requires psychological safety: reviewers must feel safe giving feedback that slows delivery, and authors must receive critical feedback without defensiveness. This comes from leadership modeling good review behavior and treating found bugs as team wins, not individual failures.

Code Review Metrics

Review turnaround time: if code sits in review for days, it creates integration conflicts and blocks the author. Target same-day reviews for urgent changes; 24-hour turnaround as a team norm.

Review comment resolution rate: what percentage of blocking comments are addressed versus explained away. Low rates may indicate insufficient context in comments.

Bugs caught per review: harder to measure but valuable signal for team effectiveness.

Avoid measuring review by comment count — it incentivizes nitpicking to show engagement.