Code Review Ethics #

There are two engineering teams that both do code review for every PR. The first team: reviews run fast, comments flow, technical discussion happens, and engineers feel smarter after every review — both as author and reviewer. The second team: reviews feel like hearings, authors get defensive reading comments, reviewers feel their reviews are ignored, and over time people start avoiding giving meaningful feedback for fear of conflict.

The difference between them isn’t tooling, isn’t formal process, and isn’t how senior the engineers are. The difference is ethics — a set of principles and behaviors determining whether the code review process becomes a powerful collaboration tool or a source of friction eroding trust.

Code review is both a technical process and a social process. Technically correct comments can still be damaging if delivered in a belittling way. Well-intentioned feedback can still trigger defensiveness if the framing is wrong. And an unhealthy review culture can make talented engineers choose silence over contribution — ultimately harming the whole team.

This article covers code review ethics: why they matter, how unethical patterns emerge and damage, and how to build conditions where code review can be what it should be — a shared investment in system quality and team growth.

Why Ethics Determine Code Review Success #

A review’s technical quality isn’t determined only by the accuracy of its comments, but also by the likelihood that those comments will be accepted, considered, and produce good changes. Correct comments delivered in a belittling way are less likely to produce productive discussion than comments that may be less precise but delivered with respect.

graph LR
    subgraph Unethical
        A1[Belittling comments] --> B1[Defensive author]
        B1 --> C1[Unproductive discussion]
        C1 --> D1[Engineers reluctant to give feedback]
        D1 --> E1[Review becomes a formality]
        E1 --> F1[Code quality declines]
        F1 --> A1
    end

    subgraph Ethical
        A2[Respectful comments] --> B2[Open author]
        B2 --> C2[Quality technical discussion]
        C2 --> D2[Both parties learn]
        D2 --> E2[Review feels valuable]
        E2 --> F2[Collaboration culture grows]
        F2 --> A2
    end

The cycles above show something important: ethics in code review are a long-term investment. One belittling comment may not immediately damage a working relationship, but the accumulating pattern — reviewers consistently belittling, authors consistently defensive — turns code review from a quality improvement mechanism into a stress source everyone wants to avoid.

Five Patterns That Damage Code Review Ethics #

1. Review as a Superiority Showcase #

This is the most team-culture-damaging pattern. Reviewers use their gatekeeper position to demonstrate technical knowledge — not to help improve code quality. Comments like “you should know this” or “this is basic stuff” create hierarchies that psychologically damage more junior engineers.

Comment comparison — superiority vs collaborative:

  Situation: the author uses a for loop where a functional approach is more idiomatic

  ✗ Superiority:
  "You don't even know this? This should use map/filter."
  → Belittling, doesn't explain why, doesn't teach

  ✗ Subtle superiority:
  "Why not use a more modern approach?"
  → Implicitly belittling without explanation, the author must guess

  ✓ Collaborative:
  "Consider using map() here — it's more expressive and
   more consistent with how we write data transformations in
   other files. Here's a short example: [example code]
   Is there a reason you chose a loop here?"
  → Explains why, gives an example, opens dialogue

2. Excessive Nitpicking #

Reviewers spending most of their energy commenting on trivial things — spaces, already-clear naming, or style preferences not in the guidelines — while missing more important architecture or logic problems. This isn’t just a waste of time; it also signals that small things matter more than substance.

Example of disproportionate nitpicking:

  PR: a payment retry mechanism implementation (200 lines)
  Comments the author receives:
  1. "The variable name 'retryCount' should be 'retryAttempts'"
  2. "There's an extra space on line 47"
  3. "Prefer single quotes over double quotes"
  4. No comments about: no jitter in the backoff,
     no circuit breaker, no metric for the retry rate

  What happens:
  → The author spends time changing 3 trivial things
  → Performance and reliability bugs slip through unnoticed
  → The review process feels like a spell-checker, not a quality gate

Simple rule: if your comment doesn’t significantly affect correctness, security, or maintainability, ask yourself whether it’s worth making. If yes, explicitly mark it [nitpick] so the author knows it isn’t a blocking issue.

3. Imposing Personal Preferences #

“I prefer this way” isn’t a valid basis for blocking a merge. Personal preferences unsupported by team guidelines, clear technical reasons, or data are a form of abused power.

Personal preference vs valid technical reasons:

  ✗ Invalid personal preferences as blocking:
  "I prefer async/await over promise chaining"
  "I usually write if-else instead of ternary for cases like this"
  "I think the singleton pattern is cleaner here"
  → All these are preferences that might be valid but shouldn't
    block merges if no guideline states them

  ✓ Valid technical reasons:
  "We agreed in ADR-023 to use async/await
   for consistency across the codebase"
  "The ternary here gets hard to read because the condition is complex —
   if-else is clearer for this case because..."
  "The singleton here makes unit testing hard due to global state.
   Dependency injection is more testable because..."
  → Supported by team decisions or criticizable technical reasons

4. Reviewing Without Reading Context #

Reviewers opening the “Files changed” tab directly without reading the PR description, ticket, or related RFC — then giving comments irrelevant to the problem being solved. This isn’t just ineffective; it also wastes the author’s time explaining context that should already be available.

The impact of context-free reviews:

  PR: "feat: implement idempotent payment processing using Redis"
  Description: explains the double-charge problem, the idempotency
  key approach with Redis, and why Redis was chosen over the DB

  Comments without reading context:
  "Why use Redis? Couldn't it just be done in the database?"
  → This question is already answered in the PR description
  → The author must re-explain something already written
  → Frustration because the reviewer didn't read

  Comments after reading context:
  "I've read about the idempotency key approach. One thing to
   consider: what if Redis goes down? Is there a fallback
   or do we accept this risk?"
  → A relevant question adding value

5. Approving Without Meaningful Review #

“LGTM” given without truly reading the code is a form of dishonesty in the review process. It creates a false sense of security — the author feels the PR was validated when it wasn’t. When bugs later appear in production, nobody can be trusted to have seen them.

Signs of meaningless approvals:

  ✗ LGTM within 2 minutes for a 400-line PR
  ✗ Approval without a single comment on a complex PR
  ✗ "Looks good, minor nitpick" then approving without checking logic
  ✗ Approving due to deadline pressure, not because it was reviewed

  Consequences:
  → Bugs reach production that should have been caught
  → The reviewer can't be trusted as a quality gate
  → Code review loses its value as a quality control mechanism

Ethical Principles That Build a Healthy Culture #

Assume Good Intent (Charitable Interpretation) #

Every engineer creating a PR does so in good faith — they want to solve a problem, not deliberately write bad code. When you see code that looks wrong, the first question shouldn’t be “why did they do this?” but “what might they have been thinking when writing this?”

Charitable interpretation in practice:

  Situation: the author uses an approach that looks over-engineered

  Negative interpretation (unethical):
  "They definitely didn't think this through"
  → Immediately judging without understanding context

  Charitable interpretation (ethical):
  "Maybe there's a constraint or requirement I don't know about
   that makes this approach reasonable. I should ask."
  → Opens dialogue: "I notice this approach is fairly complex —
     is there a specific constraint making it necessary?
     If not, there might be something simpler..."

Separate “Bad Code” from “Bad Person” #

Suboptimal code isn’t a permanent reflection of someone’s ability or character. All engineers have written suboptimal code — because of time pressure, lack of context, or not yet knowing a better way. Good feedback shows the path toward better code, not using current code quality as a judgment of the person.

Language that separates code from person:

  ✗ Language attacking the person:
  "You should know this isn't safe"
  "This is a naive approach"
  "You didn't think about this edge case"

  ✓ Language focused on the code:
  "This part could be vulnerable to a race condition if..."
  "This approach hasn't considered the case when..."
  "There's an edge case to handle here: when X happens, Y..."

Give Context, Not Just Instructions #

Comments saying “change this to X” teach nothing. Comments explaining why a change is needed, what the consequences of the current approach are, and how the alternative is better — those build the author’s ability to make better decisions in the future.

Instructions vs contextual explanations:

  Situation: the author doesn't use parameterized queries

  ✗ Instruction without context:
  "Use prepared statements."

  ✓ Contextual explanation:
  "String concatenation directly in this query is vulnerable to SQL injection —
   if the email contains characters like ' or --, the query could be manipulated.
   Use a parameterized query:

   // CURRENT ANTI-PATTERN:
   db.query('SELECT * FROM users WHERE email = ' + email)

   // CORRECT:
   db.query('SELECT * FROM users WHERE email = ?', [email])

   This is also more efficient because the database can cache the query plan."

Respect the Author’s Time Constraints #

Not all feedback needs immediate action. Good suggestions that don’t need doing before the merge should be clearly marked as non-blocking — and reviewers shouldn’t hold approval hostage just because an unimplemented suggestion exists.

Distinguishing feedback priority:

  [blocking] — must be resolved before merging:
  Logic bugs, security vulnerabilities, or serious architecture violations.
  "This will cause data loss if the transaction is rolled back."

  [suggestion] — strongly recommended, can be followed up in the next PR:
  "Consider extracting this logic into a separate function for better
   readability. Doesn't need to be in this PR, but worth noting."

  [nitpick] — minor preference, no need to act:
  "[nitpick] 'processData' could be more descriptive, but this is minor."

  With clear labels, authors can prioritize their time
  and not feel every comment must be resolved immediately.

Psychological Safety in Code Review #

Psychological safety — the condition where someone feels safe taking interpersonal risks like speaking up, asking questions, or making mistakes without fear of embarrassment — is the foundation of effective code review.

Teams with high psychological safety have engineers who:

  • Aren’t afraid to submit PRs for fear of criticism
  • Dare to show ignorance through questions in comments
  • Can disagree with senior reviewers without fearing consequences
  • Willingly give honest feedback without worrying about damaging relationships

Teams without psychological safety show different patterns:

  • PRs get made huge because engineers are reluctant to be reviewed often
  • Authors agree with every comment without truly understanding them
  • Reviewers hold back feedback that might create conflict
  • Code review becomes a ritual without substance
flowchart TD
    A{"Psychological Safety\nin Review"}

    A --> |High| B["Engineers dare to submit\nsmall, frequent PRs"]
    B --> C[Faster, more focused reviews]
    C --> D[More frequent, more specific feedback]
    D --> E[Engineers grow faster]
    E --> F[More productive team]

    A --> |Low| G["Engineers avoid review\nor make big, rare PRs"]
    G --> H[Reviews rare and heavy]
    H --> I[Feedback scarce and shallow]
    I --> J[Engineers grow slowly]
    J --> K[Defensive, less innovative team]

Building psychological safety isn’t a big project — it’s built from every small interaction. One belittling comment can damage trust built across many good reviews. One sincere appreciation can encourage a junior engineer to contribute more boldly.

Handling Disagreement Constructively #

Disagreement in code review is healthy — it means there are different perspectives worth considering. What determines whether disagreement is productive or damaging is how both sides handle it.

Patterns of constructive disagreement:

  Step 1: Make sure you understand the other side's position
  "If I understand correctly, you chose this approach because X.
   Is my understanding right?"

  Step 2: Explain your concern specifically
  "My concern is that with this approach, we'll face Y
   if Z happens. This already happened in PR 1234."

  Step 3: Offer alternatives or data
  "Have you considered approach B? Its strengths are...
   Its weaknesses are... What do you think about the trade-off?"

  Step 4: If still disagreeing, escalate properly
  "We have different views on this. Is there someone else
   whose perspective we could ask? Or should this become an RFC?"

  What must NOT be done:
  ✗ Keep debating without progress in the same thread
  ✗ Escalating the tone to the personal
  ✗ Unilaterally approving without clear resolution
  ✗ Letting disagreement block merges without a clear path

There’s a useful principle for unresolved disagreement: “disagree and commit”. If after sufficient discussion there’s no clear resolution, the team can decide to follow the author’s opinion (since they know the context best), record the dissenting opinion in a PR comment, and move on. This is better than a PR stuck for weeks because nobody will yield.

Senior Reviewers’ Responsibility Toward Junior Engineers #

More senior reviewers carry extra responsibility — not just maintaining code quality, but also maintaining conditions where junior engineers can grow. How a senior reviews a junior’s PR determines whether that junior feels supported or feels incapable.

Reviews supporting junior growth:

  ✓ Explain the "why" behind every piece of feedback
    → Juniors learn principles, not just follow instructions

  ✓ Show appreciation for what's already right
    → "[praise] The error handling here is already very good —
       you've handled all the relevant edge cases."

  ✓ Distinguish between "this is wrong" and "there's a better way"
    → "This works correctly, but there's a more idiomatic way
       you should know for the future: [explanation]"

  ✓ Ask questions instead of directly giving answers
    → "What would happen if we called this function with
       empty input?" — let the junior think for themselves

  ✗ What to avoid:
    → "You should know this at your level"
    → Comments making juniors feel they don't belong on the team
    → Different standards between seniors and juniors without explanation

Building a Healthy Review Culture #

Culture isn’t built by one person or one policy. It’s built by the accumulation of daily behaviors from all team members.

Steps for building a healthy review culture:

  1. Create explicit code review guidelines
     → What must be commented, what shouldn't
     → Label conventions (blocking, suggestion, nitpick)
     → Response time expectations from both parties

  2. Team leaders must model the expected behavior
     → Senior developers giving feedback with respect
       become models for others
     → One toxic senior can damage the entire team culture

  3. Normalize appreciation
     → "[praise]" isn't small talk — it's meaningful communication
     → Teams that appreciate each other find criticism easier to accept

  4. Create space for reflection
     → Occasionally discuss: "how is our review process going?"
     → Special retrospectives about code review after big sprints

  5. Address negative patterns immediately
     → If a reviewer is consistently belittling, this needs
       direct discussion — not ignoring
     → "Your comments on Ali's PR yesterday felt a bit harsh.
       Could you try a more supportive approach?"

Anti-Patterns to Avoid #

From the reviewer side:

✗ "You should have used this approach from the start"
  → Judging past decisions without understanding context.
  Existing decisions may have been made under constraints no longer relevant.

✗ Sarcastic or ironic comments
  → Text has no tone of voice — sarcasm in text is often read
  far more sharply than intended. Avoid it entirely.

✗ Approving PRs out of deference to a senior author
  → This is a form of social pressure damaging code review value.
  Reviews must be based on code quality, not social hierarchy.

✗ Comparing the author to others
  → "Person A never does this" or "Compare with Budi's PR"
  are unfair comparisons damaging team relationships.

────────────────────────────────────────────────────────────────────────────

From the author side:

✗ Immediately ignoring all reviewer comments
  → Every comment deserves a response — even if you disagree.
  Ignoring without response is a form of disrespect.

✗ Reacting emotionally to technical feedback
  → "This is a personal attack" when feedback targets the code.
  Separate your identity from the code you write.

✗ Merging PRs without resolving all blocking comments
  → This violates reviewer trust and damages the review process's value.

✗ "I've been an engineer for 10 years, I know better"
  → Seniority doesn't exempt anyone from code review.
  Even the most senior engineers can have blind spots.

Code Review Ethics Checklist #

AS A REVIEWER:
  □ Read the PR description before opening the diff
  □ Every comment explains why, not just what
  □ No comments readable as personal attacks
  □ Blocking and non-blocking comments distinguished with labels
  □ Appreciation given for what was done well
  □ Personal preferences not in guidelines marked as nitpicks
    or not commented on at all
  □ Disagreements delivered with questions or data, not statements
  □ Never approved without truly reading the PR

AS AN AUTHOR:
  □ Feedback received openly — question with arguments, not emotion
  □ Every comment responded to, even those producing no changes
  □ No merging before all blocking comments are resolved
  □ If disagreeing, counter-arguments delivered with data or reasoning

AS A TEAM:
  □ A documented, mutually agreed code review guideline exists
  □ Unhealthy review patterns discussed directly, not left alone
  □ Appreciation is a normal part of the review process, not awkward
  □ Juniors feel safe asking questions and making mistakes in the review process

Summary #

  • Ethics determine whether code review becomes an investment or a burden — technically correct comments delivered belittlingly produce worse results than less precise comments delivered with respect.
  • Assume good intent — engineers create PRs in good faith. The first question when seeing seemingly wrong code isn’t “why did they do this” but “what might make this reasonable?”
  • Separate the code from the person — “this code could cause problems” is very different from “you didn’t think about this”. One criticizes code, the other criticizes the person.
  • Give context, not just instructions — “use X” teaches nothing. Explaining why X is better and what the current approach’s consequences are builds the author’s ability to make better decisions in the future.
  • Disproportionate nitpicking damages the process — energy spent on spaces and variable names is energy taken from checking logic and security. Mark nitpicks as nitpicks, don’t make them blocking.
  • Personal preferences aren’t a blocking basis — without team guidelines or clear technical reasons, personal preferences shouldn’t block merges.
  • Psychological safety is the foundation — teams whose engineers fear review or fear giving feedback will produce code reviews that are formalities without real value.
  • Disagreement is healthy, but must be managed — use questions, data, and concrete alternatives. If there’s no resolution, “disagree and commit” beats a PR stuck for weeks.
  • Seniors carry extra responsibility — how a senior reviews a junior’s PR determines whether the junior feels supported or feels incapable. This is an investment in long-term team growth.
  • Culture is built from every small interaction — one belittling comment can damage trust built across many good reviews. Consistent ethical behavior is what builds culture.

← Previous: Code Review Checklist   Next: Unit Test as Guard →

About | Author | Content Scope | Editorial Policy | Privacy Policy | Disclaimer | Contact