DEV Community

IndieHacker
IndieHacker

Posted on

The Art of Saying "No" in Code Review

#career #codequality #softwaredevelopment #softwareengineering

The Art of Saying "No" in Code Review: 7 Levels from Soft to Hard

There's a skill that nobody teaches in CS programs: how to reject code without making it personal.

Native English speakers learn this through years of cultural osmosis. Non-native speakers often learn it the hard way ā€” by accidentally sounding rude, or by being so polite that their blocking feedback gets ignored.

I've collected these 7 levels from real code reviews across 4 companies and 3 countries. Each level serves a different purpose. Master all 7, and your code reviews will be clearer, kinder, and more effective.

Level 0: The Positive Observation

Not saying no at all ā€” just pointing out what's good.

Nice approach here ā€” the early return keeps the happy path clean.
Enter fullscreen mode Exit fullscreen mode 
TIL about this API. Clean usage.
Enter fullscreen mode Exit fullscreen mode 
This is much more readable than the previous version. šŸ‘
Enter fullscreen mode Exit fullscreen mode

When to use: When code is genuinely good. Sounds obvious, but most reviewers only comment when something is wrong. Positive comments build trust so that your "no"s land softer later.

Non-native speaker trap: Don't write "Good job!" ā€” it sounds patronizing from a peer. Stick to specific observations: what is good and why.

āŒ Sounds off āœ… Sounds native
Good job! Nice ā€” this handles the edge case cleanly
Very good code This reads well, especially the error handling
I think this is perfect LGTM šŸ‘

Level 1: The Nitpick

"You could change this, but you don't have to."

Nit: `userCount` might read better than `cnt` here.
Enter fullscreen mode Exit fullscreen mode 
Ultra-nit: extra blank line on L42.
Enter fullscreen mode Exit fullscreen mode 
Style nit: we usually put the constant on the left in comparisons.
Enter fullscreen mode Exit fullscreen mode

When to use: Naming, formatting, minor style preferences. Things that don't affect functionality or correctness.

The magic word: "Nit" or "Nit:" at the start. This single word tells the author: "I noticed this, but don't block your PR on it." It's a social contract ā€” if you label something a nit, you can't later block the PR over it.

Non-native speaker trap: Don't write "You should rename this variable." Without the "Nit:" prefix, it sounds like a hard requirement.

āŒ Sounds like a command āœ… Sounds like a suggestion
You should rename this Nit: maybe fetchUser instead of getData?
Change this variable name Nit: count ā†’ activeUserCount for clarity?
This name is bad Nit: the name doesn't tell me what it counts

Level 2: The Suggestion

"Here's an alternative approach. Take it or leave it."

Consider extracting this into a helper ā€” it's used in 3 places already.
Enter fullscreen mode Exit fullscreen mode 
Another option: we could use a Map here instead of filter + find. 
Would bring this from O(nĀ²) to O(n). Up to you.
Enter fullscreen mode Exit fullscreen mode 
Have you considered using early return here? Might reduce the nesting.
Enter fullscreen mode Exit fullscreen mode

When to use: When you see a better approach but the current one isn't wrong. The author should feel free to disagree.

Key phrases:

  • "Consider..." ā€” the Swiss army knife of suggestions
  • "Another option:" ā€” explicitly frames it as optional
  • "Have you considered..." ā€” implies you're curious, not commanding
  • "Up to you" / "Take it or leave it" ā€” explicit opt-out

Non-native speaker trap: "I think you should..." is NOT a suggestion in English. "Should" carries obligation. "I think" doesn't soften it enough.

āŒ Hidden command āœ… Actual suggestion
I think you should use a Map here Have you considered using a Map here?
It would be better to extract this Consider extracting this ā€” it's used in 3 places
You need to add caching This might benefit from caching if it's called frequently

Level 3: The Question

"I don't understand this. Help me understand before I judge."

Curious: why `setTimeout` here instead of `requestAnimationFrame`?
Enter fullscreen mode Exit fullscreen mode 
Is there a reason we're not using the existing `parseDate` util? 
Might be duplicating logic ā€” or maybe I'm missing context.
Enter fullscreen mode Exit fullscreen mode 
What happens if `user` is null here? I might be wrong, but this 
looks like it could panic on L47.
Enter fullscreen mode Exit fullscreen mode

When to use: When you think something might be wrong but you're not sure. The question format gives the author an out ā€” maybe they have context you don't.

Why this is powerful: Questions are face-saving. If the author made a mistake, they can fix it without anyone explicitly saying "you're wrong." If they had a good reason, they can explain without feeling defensive.

Non-native speaker trap: "Why did you do it this way?" sounds confrontational in English, even if it's genuinely curious in your language. Add a softener.

āŒ Sounds confrontational āœ… Sounds curious
Why did you do it this way? Curious about the approach here ā€” was X considered?
Why didn't you use X? Is there a reason we're not using X? (Might be missing context)
This doesn't make sense Help me understand the flow here ā€” I'm not sure I follow
What is this for? Could you add a brief comment here? I had to read it twice

Level 4: The Concern

"I think this might cause problems. Let's discuss."

Concern: this allocates on every render. With 1000+ items, 
we might see jank. Want to memoize or move it outside the loop?
Enter fullscreen mode Exit fullscreen mode 
I'm a bit worried about the race condition here ā€” if two requests 
hit this endpoint simultaneously, we could double-write. 
Could we add a lock or use an upsert?
Enter fullscreen mode Exit fullscreen mode 
Heads up: this pattern has bitten us before in the billing module 
(see #312). Might be worth adding a guard.
Enter fullscreen mode Exit fullscreen mode

When to use: Real problems that need addressing, but where you're open to the author choosing how to fix it. You're flagging the issue, not dictating the solution.

Key phrases:

  • "Concern:" ā€” like "Nit:" but serious
  • "I'm a bit worried about..." ā€” personal concern, not accusation
  • "Heads up:" ā€” framing as a friendly warning
  • "This pattern has bitten us before" ā€” evidence-based, not opinion-based

Non-native speaker trap: "This is wrong" or "This has a bug" is too direct even when it's true. Frame it as a risk or concern.

āŒ Too direct āœ… Appropriately concerned
This is wrong I think there might be an issue here ā€” [explain]
This will crash This could panic if user is nil ā€” want to add a guard?
You have a race condition Heads up: I think there's a race condition here between X and Y
This code is inefficient Concern: this is O(nĀ²) ā€” might be slow with large datasets

Level 5: The Blocker

"This needs to change before we can merge."

Blocking: this SQL query is vulnerable to injection. 
We need to use parameterized queries here. See our guide at [link].
Enter fullscreen mode Exit fullscreen mode 
This needs to be addressed: the API key is hardcoded on L23. 
Should come from env/secrets.
Enter fullscreen mode Exit fullscreen mode 
I can't approve this as-is: the migration is irreversible and 
we don't have a rollback plan. Can we discuss before merging?
Enter fullscreen mode Exit fullscreen mode

When to use: Security vulnerabilities, data loss risks, breaking changes without migration, violations of critical conventions. Things that would cause real damage in production.

Key phrases:

  • "Blocking:" ā€” the nuclear prefix, use sparingly
  • "This needs to be addressed:" ā€” clear that this isn't optional
  • "I can't approve this as-is:" ā€” states your position, invites discussion
  • "Can we discuss before merging?" ā€” escalates to synchronous conversation

Non-native speaker trap: Two opposite mistakes here:

  1. Being too soft on blockers: "Maybe we should consider not hardcoding the API key?" ā€” NO. If it's a security issue, be direct.
  2. Using blocker language for nits: "This needs to change: rename cnt to count." ā€” NO. Save the strong language for real issues.
āŒ Too soft for a real issue āœ… Appropriately firm
Maybe don't hardcode the API key? Blocking: API key on L23 must come from env. Hardcoded secrets will hit our security scanner
I think this might have a SQL injection Blocking: SQL injection on L45. Must use parameterized queries
Perhaps we should add auth here This endpoint needs auth ā€” it's currently public and returns PII

Level 6: The Redirect

"This is the wrong direction. Let's step back."

I think we might be overcomplicating this. The existing `UserService.fetch()` 
already handles this case ā€” could we use that instead of building a new path?

Happy to pair on this if it'd help.
Enter fullscreen mode Exit fullscreen mode 
Before we go further: I chatted with @sarah and this feature is being 
redesigned in Q3. Might be worth checking if this approach still aligns. 
Can we sync briefly?
Enter fullscreen mode Exit fullscreen mode 
Stepping back a bit: the original issue (#234) was about reducing latency, 
but this PR adds a new caching layer. I wonder if we could solve it at the 
DB query level instead? Simpler and fewer moving parts.

What do you think?
Enter fullscreen mode Exit fullscreen mode

When to use: When the approach itself is wrong, not just the implementation. This is the hardest "no" because you're rejecting the author's design decision, not a line of code.

Key phrases:

  • "Stepping back a bit:" ā€” signals you're zooming out
  • "I wonder if..." ā€” softest possible way to suggest an alternative direction
  • "What do you think?" ā€” makes it a conversation, not a veto
  • "Happy to pair on this" ā€” shows you're willing to invest time, not just criticize

Non-native speaker trap: "This approach is wrong" or "You should start over" will nuke the relationship. Even if the whole PR needs to be scrapped, frame it as a conversation.

āŒ Relationship-ending āœ… Redirecting with respect
This approach is wrong Stepping back ā€” I wonder if there's a simpler path here
Start over Before we go further, can we sync on the approach? I have some ideas
This is not what we need I think the original issue might be solvable without this layer ā€” want to discuss?
Why did you build this? I might be missing context ā€” is there a reason we're not using the existing X?

The Cheat Sheet

Print this. Pin it next to your monitor. Use it every code review.

Level Prefix Meaning Example opener
0 (none) This is good "Nice ā€” this handles..."
1 Nit: Change if you want "Nit: maybe X instead of Y?"
2 Suggestion: Here's an alternative "Consider..." / "Another option:"
3 (question) Help me understand "Curious:" / "Is there a reason...?"
4 Concern: This might cause problems "I'm a bit worried about..."
5 Blocking: Must fix before merge "This needs to be addressed:"
6 (conversation) Wrong direction "Stepping back:" / "Can we sync?"

The Meta-Rules

1. Match severity to language. Don't use Level 5 language ("This needs to change") for a Level 1 issue (variable naming). Don't use Level 1 language ("Nit: maybe don't") for a Level 5 issue (SQL injection).

2. One "Blocking" per review is enough. If you have 3 blockers, something went wrong upstream (unclear requirements, no design review). Have a conversation instead of leaving 3 red flags on a PR.

3. Always pair criticism with context. "This is O(nĀ²)" is a complaint. "This is O(nĀ²) ā€” with our current dataset of 50K rows, this loop takes ~3s. Using a Map brings it to ~5ms" is actionable feedback.

4. Separate "what" from "how." Flag the problem (Level 4-5), but let the author choose the solution unless there's only one right answer. "This could panic on nil" is better than "Add a nil check on line 47" ā€” the author might have a better fix in mind.

5. If you're angry, save as draft. Come back in 30 minutes. The PR will still be there. Your relationship with the author won't survive a review written in frustration.

Why This Matters Beyond Code

Code review is one of the few places where developers give and receive direct feedback daily. How you do it shapes how people perceive you:

  • Too soft ā†’ your feedback gets ignored, bugs ship
  • Too harsh ā†’ people stop requesting your review, you lose influence
  • Just right ā†’ you become the reviewer everyone wants, and your team's code quality goes up

For non-native speakers, the stakes are higher. You're already navigating a language gap. Getting the tone wrong in code review can create a reputation problem that has nothing to do with your technical ability.

The good news: unlike grammar, code review tone is a finite skill with learnable patterns. The 7 levels above cover ~95% of situations. Practice them deliberately for two weeks and they'll become automatic.

This is part of my "English for Developers" series. Part 1: How to Write a PR Description That Actually Sounds Native

I'm building DevGlish ā€” a macOS menu bar app that gives you these kinds of context-aware English suggestions while you work. Select text ā†’ āŒ˜ā‡§D ā†’ instant guidance. It detects whether you're in GitHub, Slack, or email and adjusts the tone and phrasing accordingly.

Try it free ā†’

Top comments (0)