Split View: 가르치는 코드 리뷰, 상처 주는 코드 리뷰 — 같은 지적을 다르게 만드는 것들

Code Review That Teaches, Code Review That Wounds — What Makes the Same Note Land Differently

Introduction — Same Note, Different Outcome

Take two review comments. One is "why did you write it like this?" The other is "this function is doing validation and persistence together, so changing a validation rule later would mean touching the storage logic too. What if we split it in two?" They point at exactly the same thing. But the person who gets the first one starts preparing a defense, and the person who gets the second one looks at the code.

The difference tends to get explained as a matter of character or manners, when in fact it is mostly a matter of format. Kind people do not write good reviews; good formats extract good reviews out of ordinary people. So this post is about format rather than attitude. The bar for approval, the prefix on a comment, the order of the sentences, the size of the PR — all of them are things a team can settle in one wiki page.

The Three Purposes of Review, and the Two Most Teams Lose

Code review does three things at once. It finds defects, it spreads knowledge, and it distributes ownership of the code. Most teams treat the first as the definition of review and the other two as side effects.

What is interesting is that the data points the other way. Alberto Bacchelli and Christian Bird, in a 2013 study conducted at Microsoft, reported that while developers and managers name defect finding as the top purpose of review, the output actually observed looks different. The majority of real comments were not defect reports but readability improvements, alternative approaches, and shared understanding of why the code looks the way it does. The 2018 Google case study from Caitlin Sadowski and colleagues reached a similar conclusion. Inside Google, the motivation perceived as primary sits closer to education and norm maintenance than to hunting bugs.

The second purpose, knowledge transfer, works like this. A reviewer reading code acquires a map of that domain. Six months later, when that code blows up at three in the morning, having even one person around who reviewed it changes the recovery time. Seen this way, staffing a review with only "the person who knows it best" is not an optimization. One person who knows it well plus one person who does not is often the better trade for the team as a whole.

Third, shared ownership shows up when something goes wrong. Code that merged through review is a decision of the team rather than the property of its author. This is not a device for dodging responsibility; it is the precondition for treating failure as a systems problem rather than an individual mistake. Why a culture that stops blaming individuals is a condition for resilience is covered in turning what happened into an asset.

The Bar for Approval — Not Perfect, Clearly Better

The most common route to review hell is a reviewer asking themselves the wrong question. Ask "is this the code I would have written?" and approval never arrives.

The engineering practices document Google published sorts this out explicitly. The gist is this: a reviewer should not demand that code be perfect, and should approve when the change clearly improves the overall code health of the system. Attached to it is the line that there is no such thing as perfect code, only better code. The approval question is not whether this is the best possible version, but whether it is better than what was there before the merge.

Why this bar matters is that the cost of the opposite bar is hard to see. Under a reviewer who demands perfection, people respond in two ways, and neither is splitting PRs smaller. They start working in ways that avoid review altogether. They defer refactoring, they drop that reviewer from the reviewer list, and they look for grounds to skip approval on the excuse that something is urgent. Push review quality to the extreme and review itself gets routed around.

There are exceptions to the bar, though. Security, data integrity, migrations that are hard to reverse, the contract of a public API — "it got better, so it passes" does not hold in those areas. Agreeing as a team, in advance, on the list of what blocks and what goes through keeps that judgment from being misread as one reviewer being difficult.

The Grammar of a Comment — Severity Markers and a Three-Part Sentence

The most common reason a review hurts is not the content but the severity failing to travel. The reviewer writes something at the level of "would be nice, but fine either way," and the author reads it as "fix this or I will not approve." Attach ten comments and all ten get read at maximum severity.

The fix is startlingly simple: add a prefix. Google's review documentation spells out the convention of prefixing minor points with "nit:". Adapt the wording to your team; three levels are enough.

The real change a prefix creates is on the reviewer's side too. To attach a severity, a reviewer has to ask themselves whether this genuinely blocks a merge, and that question alone visibly reduces the number of blocking comments.

Once severity is set, the next thing is sentence structure. Write in the order observation, impact, suggestion and the judgment words drop out on their own. "This code is bad" has no observation, no impact, and no suggestion; it is pure verdict. Whereas "this function is swallowing the exception (observation), which will make tracing the cause during an incident hard (impact). What about logging it and rethrowing (suggestion)?" turns the same content into collaboration. This structure is not specific to code review; it is general feedback grammar. Sentence patterns for particular situations are collected in workplace phrases that work.

One thing to add: a comment naming what went well is information, not politeness. If only negative signal flows through review, the author has no way to know what to keep. A single line like "I would like to take this error handling approach into the other modules too" is how a team convention gets made.

Designing the PR for the Reviewer — Size and Description

A large share of review quality is determined by the author, not the reviewer. And the biggest variable the author holds is size.

There is a widely cited number: past 200 to 400 lines at a time, defect-finding ability drops. The source is a 2006 case study SmartBear ran at Cisco. Analyzing data from about 50 developers, 3.2 million lines of code, and 2,500 reviews, it concluded that defects found per line fall as review size grows, and that effectiveness drops sharply above a pace of 500 lines an hour or beyond 60 minutes of continuous review.

It is worth knowing the limits of that evidence too. It is a single case study from one company, one codebase, and the tooling environment of 2006, and it assumes practices that predate the GitHub pull request. On top of that, the headline metric, defect density, is divided by line count, so a lower value on a large change is partly true by definition. That it counted only the defects reviewers actually reported is another catch. So 200 to 400 lines is more accurately taken as a rule of thumb than a law of physics.

Even as a rule of thumb, the direction is broadly right. For reference, the median change size reported in the Google case study was in the tens of lines. Far smaller than we imagine. In practice this much is enough: do not mix refactoring and behavior change in one PR, commit automated formatting separately from real logic changes, and for large changes merge the interface first and attach the implementation behind it.

Once size is handled, the other half is the description. A good PR description carries three things: why this is happening (context and links), what options were considered and why this one was chosen (alternatives), and where you want attention (the review request). The third is what decisively cuts round trips in asynchronous review. One line saying "please look especially at the lock handling in file three, the rest is mechanical" saves the reviewer two hours and saves the team a day of round trips.

The Craft of Being Reviewed

Review culture is not built by reviewers alone. There is craft on the receiving end too.

First, delay your first reaction. Reading a comment and immediately writing a rebuttal is the most common mistake. The more time you spent on the code, the more a note reads like a personal attack. Read it again thirty minutes later and the same sentence often looks different.

Second, when you disagree, ask for the reasoning. "That would probably hurt performance" reaches a conclusion far slower than "I picked this approach because of A, so what does your suggestion buy us on that dimension?" If the argument drags, changing the format is also an option. Past three round trips, a fifteen-minute call is cheaper than twenty messages in a thread.

Third, reply even to the things you decided not to do. A comment passed over in silence reads to the reviewer as being ignored. One line does it: "I will handle this in a follow-up PR, the ticket is filed" or "this behavior is intentional, I added a comment explaining it." Response rate builds trust more than adoption rate does.

Fourth, say out loud what you learned. "I did not know this pattern, that is useful" is not flattery; it is feedback to the reviewer. Review is generally poorly rewarded labor, and that one line changes how carefully the next one gets done.

When Review Becomes a Power Game

Everything so far assumes the healthy case. The unhealthy case exists too.

The signs tend to look alike. Only one particular reviewer takes forever on your PRs. The nature of the objections changes review to review, so there is no target to hit. Architectural problems slide past while variable names and whitespace collect twenty comments. That last item has exactly the structure C. Northcote Parkinson pointed out in 1957 with the bike shed. Nobody can offer an opinion on the reactor design, but everybody has one about the color of the bike shed. Where trivial comments cluster is usually a signal that nobody read the hard part.

There is also empirical work suggesting review is not a perfectly neutral technical act. The 2017 analysis of GitHub pull requests by Josh Terrell and colleagues reported that women had a higher acceptance rate overall, but that the advantage disappeared for outside contributors whose gender was identifiable from their profile. The study drew substantial pushback over its gender inference method and over causal readings of observational data, and the question is not settled. But not being settled is not evidence for "review is purely technical" either.

The response works better as process than as personal skill. Let formatters and linters, not people, rule on style arguments. Nail severity prefixes into team rules so blocking authority is explicit. Track time-to-approval and round-trip counts as team metrics rather than per-person ones. And if a different standard repeatedly applies to one particular person, that is not a problem to solve in a review thread; it is a problem for a one-on-one with your manager.

Closing — Review Passes Through People, Not Code

The measure of a good review culture is not the number of defects found. It is how many more people can fix that code six months from now. Defects also get caught by tests and monitoring, but review is very nearly the only channel through which design context spreads across a team.

There are only two things you can change today: put a severity prefix on your next comment, and put one line in your next PR description saying where to look. Culture is not made of manifestos; it is made of lines like these stacking up.