Chapter 05

Code Review 沟通的艺术

review 是一种异步的、跨文化的、留痕的协作——每一句话都决定了你和同事的关系。

5.1 Code Review 是英语最难的工程场景

Code review 难在三个维度同时作用:

  1. 留痕。 不像口头会议讲完就过;review 评论永远在那里,会被未来的同事、招聘官、新来的实习生看到。
  2. 跨文化。 评审者可能来自任何国家,对"直白"和"礼貌"的接受度不同。德国 / 荷兰同事偏好直接,日本 / 印度同事偏好委婉,美国同事介于中间但极看重 "psychological safety"(心理安全感)。
  3. 权力不对称。 Reviewer 有 approve 权,作者要按 reviewer 意见改。语气稍重一点就让作者感觉被 patronizing(屈尊俯就)。

所以 code review 英语的核心不是单词或语法,而是分寸感——什么时候该坚持,什么时候该软化,什么时候该承认自己可能错了。

5.2 四类 Review 评论

把 review 评论按"作者必须做什么"分成四类:

类型对作者的要求典型句型
Praise(夸奖)"Nice abstraction!"
Suggestion(建议)可改可不改"Consider using X instead."
Question(问题)解释或澄清"Why do we cache here?"
Blocking(阻塞)必须改"This is a security risk."

新手 reviewer 最常犯的错误:把 Suggestion 和 Question 写成 Blocking 的语气——让作者无法判断这条意见的分量,只能默认全改,最后浪费时间。

类型 1:Praise — 夸奖

夸奖是被严重低估的 review 评论。它的作用:

真实例子:

// React PR #28910
"Love how this commit isolates the change to one file. 👍"

// Rust PR
"Nice — using `Cow` here saves an allocation in the common case."

// 普通同事 PR
"This is a really clean refactor. Reading top-to-bottom now makes sense."

"Great test coverage on the edge cases."

"TIL about std::ranges. Going to steal this pattern."

夸奖要具体。"Looks good" 比没说还差(让人觉得敷衍)。"Nice — the early return saves a level of nesting" 才有价值。

类型 2:Suggestion — 建议

建议的关键是可拒绝性——明确告诉作者"你可以不接受"。Conventional Comments 的 nitpick (non-blocking) 标签就是这个目的。

# 高频建议句型
Consider X.
Could we X?
What if we X?
Maybe we could X?
nit: prefer X.
nitpick: rename foo → bar?
Style nit: 4-space indent here.
Just a nit, feel free to ignore: X.
Optional: X would also work.
For consideration: X.

# 真实例子
nit: `users` would be more conventional than `userList`.
Consider extracting this into a helper — it's used in 3 places.
Could we use the new `URL.parse()` here? Slightly cleaner.
What about a `Result<T, E>` instead of throwing?

类型 3:Question — 问题

当你不确定作者为什么这么做时,问问题永远比断言要好——它给作者面子,也给你下台阶。

# 真诚提问
Why do we need this lock here? The map is goroutine-local, isn't it?
What's the rationale for choosing 30 seconds?
Is there a reason we don't reuse the existing `parseConfig`?
Could you walk me through how this handles the empty case?

# 假装不懂的提问(其实知道答案,让作者自己发现问题)
What happens if `items` is nil?
What's the behavior when N exceeds the buffer size?
Have we considered the case where two callbacks fire at once?

第二种 "苏格拉底式" 提问比直接说 "this is buggy" 体验好得多——作者自己想明白,更愿意改。

类型 4:Blocking — 阻塞合并

阻塞性意见必须明确,不要用问号或建议语气包装:

# Good — 明确阻塞
This will crash on empty input. We need to handle that before merging.
Blocking: this introduces a SQL injection vector. Use parameterized query.
This breaks the contract documented in `docs/api.md`. We can't change
this without a migration plan.

# Bad — 用建议语气包装阻塞性意见
"Maybe we should validate the input?"  ← 听起来可改可不改
"Consider parameterizing the query"     ← SQL 注入是 must fix

用 Conventional Comments 标签可以让阻塞性更明显:

issue (blocking): SQL injection vector — use parameterized query.
issue (security): this leaks the API key into logs.
issue: this breaks BC; needs an entry in CHANGELOG.

5.3 软化语气的 Hedging 技巧

Hedging(措辞软化)是 reviewer 必备技能。它不是委婉得让对方看不懂你在说什么,而是给对方留余地

Hedging 公式

[认知前提] + [核心建议] + [开放结尾]

三个组件:

  1. 认知前提:表达"我可能错"或"我可能没看全"
  2. 核心建议:你的实际想法
  3. 开放结尾:邀请讨论

认知前提常用句型

I might be missing context, but ...
Possibly a dumb question, but ...
I haven't traced through this fully, but ...
Take this with a grain of salt — ...
Not sure if this matters here, but ...
Could be wrong, but ...
For my own understanding — ...
I'm probably missing something obvious — ...

开放结尾常用句型

... what do you think?
... happy to discuss.
... let me know if I'm off base.
... feel free to push back.
... if you've already considered this, ignore me.
... thoughts?
... wdyt? (what do you think — 缩写)

合在一起的实例:

"I might be missing context here, but it looks like the cache key
doesn't include the user's locale. If two users with different
locales request the same resource, would they share the cache?
Wdyt?"

# 翻译 = "我可能没看全,但 cache key 好像没包含 locale。
# 不同 locale 的用户会不会撞 cache?你怎么看?"

这一段同样的意思直白说就是 "Cache key is wrong"——但加了 hedging 后,对方更愿意接受。

5.4 提出反对意见的高级用法

"反对" 比 "建议" 难。当你和作者意见冲突时,需要既坚持立场又不冒犯。

常用反对句型(按强度递增)

强度句型意思
1Have you considered X?有想过另一种吗?
2I see the appeal of this, but ...我懂这思路,但是
3I'm not fully convinced because ...我还没被说服
4I'd push back on this — ...我对此持反对意见
5I think we should reconsider X.我们该重新考虑
6I'd be uncomfortable shipping this without ...没有 X 我不放心上线
7I disagree with this approach.我不同意这个做法
8This is a hard no for me — ...这一点我必须说不

使用规则:从低强度开始,逐渐升级,最后才用 7-8

反对意见的完整结构

"I see your point about [作者立场], but I'd push back on this.

[原因 1:技术理由]
[原因 2:维护成本/团队一致性/历史经验]

[替代方案]

Happy to jump on a quick call if it's easier to discuss live."

实例:

"I see your point about reducing boilerplate, but I'd push back on
introducing a new DSL here.

We've been burned twice in the past with custom DSLs (config-v1,
template-engine-v2) — every junior who joins has to learn it from
scratch, and the maintenance burden falls on whoever wrote it.

A plain function-based API would give 80% of the ergonomic benefit
without the learning cost. Something like:

  pipeline.add(stepA).add(stepB)

Happy to mock this up and compare side by side. Wdyt?"

这段反对意见做对了 4 件事:(1) 先承认对方的合理性,(2) 给具体历史证据,(3) 提替代方案,(4) 邀请进一步讨论。

5.5 接受 review 的回应模板

作为 PR 作者收到评论时,有 5 种典型回应:

1. 接受并已修改

Done. Renamed in <commit-sha>.
Good catch! Fixed in latest push.
You're right, much cleaner. Updated.
Done — also added a test to lock in the behavior.
Fair point, fixed.

2. 接受但改在另一个 PR

Agreed this needs work, but it's pre-existing — out of scope here.
Filed #4998 to track.

Good idea. I'll handle this in a follow-up PR after this lands.

This is a great suggestion but a bigger refactor than I want to
include here. Created issue #5001 for it.

3. 不同意 — 解释原因

I considered that, but went with the current approach because
<reason>. Happy to discuss further if you feel strongly.

I'd actually prefer to keep it as is — the redundancy is intentional
to avoid the import cycle we hit in #3421.

Hmm, I see the appeal but I think the explicit version is easier
for new readers. Could you sell me on the change?

4. 不确定 — 推迟决定

Let me sleep on this and get back to you tomorrow.
Going to think about this — will reply by EOD.
@reviewer1 thoughts on this thread? Two of us are split.

5. 完全同意但还没改

Will fix in next push.
Will address — currently working through the other comments.
Got it, on it now.
Acknowledged 👍 — pushing fixes shortly.

5.6 LGTM 系列高频缩写

这些缩写在英文 code review 里几乎是黑话,看不懂会很吃亏:

缩写全写含义
LGTMLooks Good To Me同意合并
SGTMSounds Good To Me同意(对方案/想法)
WFMWorks For Me对我可行
IIRCIf I Recall Correctly如果我记得没错
IIUCIf I Understand Correctly如果我理解正确
AFAIKAs Far As I Know就我所知
AFAICTAs Far As I Can Tell从我能看到的
TILToday I Learned今天学到了
TL;DRToo Long; Didn't Read简短版
WIPWork In Progress仍在进行
NIT / nitnitpick吹毛求疵的小问题
PTALPlease Take Another Look请再看一下(修改后)
PTAL / RFALReady For Another Look同上
WDYTWhat Do You Think你怎么看
FWIWFor What It's Worth仅作参考
FYIFor Your Information供你参考
MRE / MCVEMinimal Reproducible Example最小可复现例子
OOOOut Of Office请假/不在
EODEnd Of Day下班前
EOWEnd Of Week本周末前
NBDNo Big Deal小事一桩
YMMVYour Mileage May Vary因人而异
BCBackwards Compatibility向后兼容
BTWBy The Way顺便说
IMO / IMHOIn My (Humble) Opinion我的观点
// trap

LGTM 在 GitHub 上几乎等于 approve,但严格来说是正式 approve——你必须点 "Approve" 按钮才生效。某些团队把 "LGTM" 写在评论里被视为 "I'd approve once X is done"。

5.7 30 个真实 GitHub Review 评论拆解

从 React 仓库(5 条)

1. "I think this might run into the same issue we hit in #28392.
    Could we use `useSyncExternalStore` instead?"
    分析:用 "I think... might"(hedging) + 引用历史 issue + 提替代方案

2. "nit: there's a typo in this comment, but otherwise lgtm"
    分析:nit 标签 + 直接 LGTM(不阻塞)

3. "Why is this `useEffect` and not `useLayoutEffect`? The DOM read
    happens before paint."
    分析:苏格拉底式提问,让作者自己想明白

4. "I'm not sure I understand the second case. Could you walk me
    through it?"
    分析:把"代码不清楚"包装成"我不懂"

5. "Strong +1 on the API shape. Bikeshed: `defer` vs `delayed`?"
    分析:Strong +1 表赞同;Bikeshed 表示 "我知道这是小事但..."

从 Rust 仓库(5 条)

6. "This is a heavy hammer for a small nail. Can we get away with
    just bumping the version check?"
    分析:使用比喻 "heavy hammer for small nail"

7. "AFAICT this changes the semantics of `take` for owned values.
    That'd be a soundness bug, no?"
    分析:AFAICT 软化 + 反问 "no?"

8. "Looks reasonable. Could you add a test that exercises the edge
    case where `n == 0`?"
    分析:先肯定再要求

9. "I would expect this function to be `unsafe` given it dereferences
    a raw pointer. Am I missing something?"
    分析:先陈述期待 + "Am I missing something" 给对方台阶

10. "r? @ghostowner"
    分析:r? = request review from,重定向 reviewer

从 Linux kernel mailing list(5 条)

11. "NAK. This breaks the API contract documented in
    Documentation/dev-tools/...".
    分析:NAK = Not Acknowledged,正式拒绝

12. "Reviewed-by: Linus Torvalds <torvalds@linux-foundation.org>"
    分析:仅一行表示 review 通过

13. "I'd rather see this implemented as a sysfs interface, but I won't
    block on it. Please add a comment explaining why ioctl was chosen."
    分析:明确"不阻塞"+ 妥协请求

14. "This patch is doing two things — please split it."
    分析:直接命令式(kernel 风格更直接)

15. "Tested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>"
    分析:Tested-by 表示亲自测过

从 Vue / Vite 仓库(5 条)

16. "Could we add a regression test? It's small enough that I want
    to make sure we never re-introduce the bug."
    分析:解释 "为什么要加测试"

17. "I love the idea but I'm worried about bundle size implications.
    Did you measure the delta?"
    分析:先表赞许,再提出具体担忧

18. "Hmm, this feels like the kind of thing that should be in a plugin
    rather than core. Thoughts?"
    分析:feel like + thoughts 表示 "我直觉,你怎么看"

19. "We've explicitly avoided this pattern in the past — see commit
    abc123. What changed?"
    分析:引用历史 + 反问

20. "+1 from me. Deferring to @yyx990803 for the final call."
    分析:表态后让出最终决定权

同事日常 review(5 条)

21. "Just a heads-up: the staging DB schema doesn't have this column
    yet. Want me to file the migration?"
    分析:heads-up = 提个醒;提供帮助

22. "Pulled this down locally and ran the tests — all green. LGTM 🚀"
    分析:表示亲自测过 + emoji 增加亲和力

23. "I'll let you decide — both work. Happy either way."
    分析:明确 "你决定",避免反复来回

24. "Mind explaining why we're not using the existing `processBatch`?
    Genuine question."
    分析:Genuine question 强调 "不是反对"

25. "This is great work. Two small things and an optional thought below."
    分析:开篇定调 + 总览三类评论

新人对资深的 review(5 条 — 反过来)

26. "Forgive the naive question, but: why do we need a separate
    process for this?"
    分析:Forgive the naive question 是经典前缀

27. "I'm new here so I might be missing context, but ..."
    分析:先表明立场

28. "I tried to follow the logic but got lost around line 142. Could
    you point me to where the lock is released?"
    分析:把 "代码难懂" 转化为 "我能力不够"

29. "Sorry to delay merging — one more question if you don't mind."
    分析:先道歉再提问

30. "Pinging @senior-dev to gut-check before I approve."
    分析:让出决定给资深,避免越权

5.8 跨文化沟通的几条规则

  1. 不要用 "you should" / "you must"。改成 "we could" / "consider"。
  2. 不要写情绪词。"This is wrong" / "This is bad" 会引发对抗;"This won't work because X" 是讨论事实。
  3. 多用 "we",少用 "you"。"我们要不要..." 比 "你应该..." 友好十倍。
  4. 给具体的修改建议。"This is unclear" 不如 "Consider renaming `foo` to `userId`"。
  5. 承认对方的努力。任何 PR 都意味着对方花了时间。一句 "Thanks for tackling this" 不会让你掉价。
  6. 面对面解决冲突。如果一条评论来回 3 次以上还没共识,立刻 "Let's hop on a call"。
// danger

很多中国程序员习惯了国内 "直接=高效" 的工程文化,到海外团队会被同事投诉 "rude"。一个简单测试:把你的评论换成同事写给你的,你听了会舒服吗?如果不会——就重写。

5.9 本章小结

下一章我们换到"输入"端——技术文档阅读策略。