代码审查攻防能力
# 05.代码审查攻防能力
# 目录介绍
- 1. 案例引入
- 2. Code Review 全景图
- 3. 收 Diss 五级分类
- 4. 被 Diss 的应对姿势
- 5. 评审别人的黄金句式
- 6. Review 分层策略
- 7. 大 PR 的评审技巧
- 8. Review 反模式
- 9. 把 Review 变教学场
- 10. 综合案例串讲
# 1. 案例引入
# 1.1 被 Diss 17 次的 PR
张伟支付重构进入编码阶段, 第一周他就交出了一个大 PR——支付路由算法核心 300 行。信心满满,自我 review 三遍才提。
结果 PR 上线 4 小时,已经收到 17 条评论,来自 5 位不同架构师:
评论 1 (架构师 A): "这个 if-else 太长, 改用策略模式"
评论 2 (架构师 A): "变量名 c 是什么? 至少写 channel"
评论 3 (架构师 B): "为什么不用 Guava Cache?"
评论 4 (架构师 B): "这段和你 3 个月前那个 PR 一模一样, 不能抽出来吗?"
评论 5 (架构师 C): "线程安全没考虑, 你这个 map 是 HashMap"
评论 6 (架构师 C): "test 覆盖率不够 40%"
评论 7 (架构师 D): "命名不符合我们的 style guide, 见 P.12"
评论 8 (架构师 D): "这个方法太长, 拆成 3 个"
评论 9 (架构师 E): "为什么不用 Optional 而用 null 判断?"
评论 10 (架构师 A): "构造函数参数太多, 用 Builder"
评论 11 (架构师 B): "Log 级别写错了, 这应该是 warn 不是 info"
评论 12 (架构师 C): "SQL 拼接有注入风险, 用 PreparedStatement"
评论 13 (架构师 D): "这段代码和 3 年前老王写的 PayHelper 有点像"
评论 14 (架构师 E): "你这写法有点像刚入行的......"
评论 15 (架构师 A): "package 结构不对, 应该按 DDD"
评论 16 (架构师 B): "@张伟 你确定这个业务场景是这么定义的?"
评论 17 (架构师 A): "综上, 建议这个 PR 整体推翻重写"
结论:张伟看完评论,五味杂陈。其中有真正的技术问题, 有 style 挑剔, 有 3 年前的旧账, 还有阴阳怪气。他花了 6 小时逐条回复,越回越气,最后差点删库跑路。
3 天后 PR 还没合并,他找 Leader 老王吐槽:
张伟: "老王, 我这 PR 是真有那么烂吗?"
老王: (看了看) "有的评论确实要改, 有的其实可以怼回去,
有的是这几位架构师之间的口水战, 跟你无关。
你分不清哪个是哪个, 一律全改,
所以 3 天了还没进展。"
张伟: "那我该怎么应对?"
老王: "你要学会给每条评论定级——什么必须改、什么可以让、
什么必须怼、什么请上级仲裁。
不是所有 Diss 都值得回应, 也不是所有意见都必须接受。"
# 1.2 顺藤摸到根因
带着这条线往下挖:
- 假设 1:张伟代码烂?——不是,方案通过、逻辑正确
- 假设 2:架构师故意刁难?——部分,但大部分是善意的
- 假设 3:那问题在哪?——评论没分级 + 应对没策略
- 假设 4:为什么老手能处理 17 条不崩?——他们有分级机制
- 假设 5:什么是分级机制?——Nit/Question/Suggestion/Blocker/Attack 五级
- 假设 6:每级怎么应对?——不一样,有的接、有的驳、有的升级
- 假设 7:这些机制是什么?——本篇的主线
仔细分析张伟这次翻车的根因:
表象 真实问题
────────────────────────────────────────────
17 条一起看崩溃 → 没分级, 大脑处理不了 17 个并行冲突
每条都逐字回复 → 没策略, 时间被稀释到 nit 上
"我改我改我都改" → 无原则让步, 失去自主权
"你这写法像刚入行" → 被攻击后没反击, 心态崩了
3 天不 merge → 没主动 escalate 到老王仲裁
根因:张伟以为"Code Review = 别人挑错我改错"——其实 Code Review = 集体优化代码的协作博弈。博弈就有策略,不该全盘接受。
这一段里至少藏着 8 个原理点:
① Code Review 到底要达成什么? 就是挑错吗? → 第 2 章
② 17 条评论怎么分类才不崩? → 第 3 章
③ 每种评论怎么应对? → 第 4 章
④ 评审别人时怎么措辞才不树敌? → 第 5 章
⑤ 大 PR 300 行怎么评审才不漏? → 第 6-7 章
⑥ Review 到底谁应该看什么? → 第 6 章
⑦ 常见的 Review 反模式有哪些? → 第 8 章
⑧ Review 能不能变成团队成长机会? → 第 9 章
# 1.3 我们要回答什么
这个事故就是本篇的主线案例。带着上面 8 个问号往下走。
本篇路线:
Review 全景图 (第 2 章)
↓
收 Diss 五级 (第 3 章) ─→ 解开"分类"
↓
被 Diss 应对 (第 4 章) ─→ 解开"分类后怎么办"
↓
评审黄金句式 (第 5 章) ─→ 解开"评别人不树敌"
↓
分层策略 (第 6 章) ─→ 解开"谁看什么"
↓
大 PR 技巧 (第 7 章)
↓
反模式 (第 8 章)
↓
把 Review 变教学场 (第 9 章)
↓
综合案例 (第 10 章) ─→ 张伟的第 18 次评论
📌 本篇定位:Code Review 是日更的战场——每周你会经历数十次。这些 daily interaction 累积决定了你在团队中的位置。评审能力好的人,3 年后是 Tech Lead;评审能力差的,3 年后还在写 CRUD。
# 2. Code Review 全景图
# 2.1 五种 Review 目的
Code Review 不是一件事,是五件事:
┌────────────────────────────────────────────────────────┐
│ Code Review 的五种目的 │
├────────────────────────────────────────────────────────┤
│ │
│ ① 缺陷发现 (Defect Detection) │
│ 找 bug, 找性能问题, 找安全隐患 │
│ │
│ ② 知识共享 (Knowledge Sharing) │
│ 让更多人熟悉这块代码, 打破知识孤岛 │
│ │
│ ③ 风格对齐 (Style Alignment) │
│ 命名、格式、包结构统一 │
│ │
│ ④ 教学指导 (Mentoring) │
│ 资深带新人, 传授最佳实践 │
│ │
│ ⑤ 集体所有权 (Collective Ownership) │
│ "代码是团队的, 不是你的"——避免单点 │
│ │
└────────────────────────────────────────────────────────┘
对照张伟的 17 条评论,拆解如下:
① 缺陷 (要改): 5, 11, 12 (线程安全/日志/SQL注入)
② 知识 (可讨论): 4, 13 (是否可复用旧代码)
③ 风格 (小改): 2, 7, 8, 15 (命名/style/结构)
④ 教学 (选择性): 1, 3, 9, 10 (策略模式/Guava/Optional/Builder)
⑤ 集体 (讨论): 6 (测试覆盖率)
攻击 (要顶): 14 (阴阳怪气)
无效 (拒绝): 17 (整体推翻建议)
过时 (拒绝): 4 (3 年前的旧账)
17 条 = 4 条必改 + 3 条可讨论 + 5 条小改 + 3 条选择性 + 2 条要驳。不是全都要接受。
# 2.2 目的错配的悲剧
疑惑:如果 Review 者和作者对"目的"理解不同,会怎样?
论证:目的错配是 Review 冲突的根源:
作者眼中: 我在做 P0 交付, Review 是"缺陷发现"
Reviewer 眼中: 我在教年轻人, Review 是"教学指导"
冲突:
- 作者想快速 merge, 只想改 bug
- Reviewer 想让作者学会最佳实践, 逐行挑
- 结果: 作者觉得"挑刺", Reviewer 觉得"不听话"
关键动作:PR 描述里主动申明本次 Review 的目的:
PR 描述模板:
## 变更目的
XX 支付路由算法, 关闭 issue #123
## Review 期望
- 主评审: 老王 (功能正确性 + 架构一致性)
- 副评审: 老李 (性能 + 线程安全)
- 建议评审: 老张 (代码风格)
## 时间紧迫度
下周三前必须上线 (灰度 M2 依赖)
## 特别请关注
- 支付路由核心算法 (PayRouter.java line 45-180)
- 缓存策略 (是否用 Guava, 请老王决策)
结论:主动申明目的 + 主评审 + 时间线——评审者按你的框架来,冲突降 80%。
# 3. 收 Diss 五级分类
# 3.1 Nit 挑刺型
特征:微小的、可改可不改的、更多是审美的。
例:
- "变量名 c 改成 channel"
- "这行末尾多个空格"
- "import 顺序不对"
特点:不改不影响功能, 改了确实更好。
分辨:评论者通常会加 nit: 前缀(业界惯例)。没加前缀但你判断是 nit 的,自己心里给个 nit 标签。
# 3.2 Question 疑问型
特征:评论者不确定,想问清楚。
例:
- "这里为什么用 HashMap 而不是 ConcurrentHashMap?"
- "这个分支什么时候会触发?"
- "为什么不用 Optional?"
特点:没有明确要求改, 只是问。
关键洞察:Question 是评论者在"帮你想", 不是"挑刺"。认真答 = 展示你的思考深度。
# 3.3 Suggestion 建议型
特征:建议 A 改成 B, 但没强制。
例:
- "建议改用策略模式"
- "可以考虑抽个 abstract 基类"
- "或许 Guava Cache 更好?"
特点:评论者认为 B 更好, 但接受 A。决策权在你。
分辨:评论者通常会加 suggestion: 或 optional: 或用"或许/可以考虑"。
# 3.4 Blocker 拦截型
特征:必须改, 否则不能 merge。
例:
- "SQL 注入风险, 必须用 PreparedStatement"
- "线程安全问题, HashMap 在并发场景必崩"
- "这里泄漏了用户手机号, 违反数据合规"
特点:涉及 bug / 安全 / 数据 / 合规 / 架构一致性——不改不行。
分辨:评论者通常语气严肃, 或明确 must fix / blocker / P0。
# 3.5 攻击型评论
特征:对人不对代码, 带情绪, 阴阳怪气。
例:
- "你这写法有点像刚入行的"
- "3 年前的老代码风格"
- "综上, 建议整体推翻重写"
特点:不是技术反馈, 是关系博弈。
关键:攻击型评论必须处理, 不能忍——一次忍让 = 后续被继续踩。但处理方式必须专业(详见 4.3)。
# 4. 被 Diss 的应对姿势
# 4.1 就事回击
疑惑:什么样的评论必须回击而不是接受?
论证:回击的三种情形:
① 评论技术上错的
评论: "HashMap 线程不安全"
现实: 这个 map 只在单线程构造后是不可变的
→ 回击, 解释场景
② 评论过时的
评论: "3 年前老王的 PayHelper 有类似逻辑"
现实: 老代码是过时的, 不复用是有意为之
→ 回击, 说明原因
③ 评论是个人偏好
评论: "我觉得应该用 Optional"
现实: Optional 在这个热路径上有性能开销
→ 回击, 拿数据
回击的姿势 — 三段式:
① 承认关切: "谢谢指出" / "这个问题我也想过"
② 陈述事实: "但实际上 X 因为 Y" (给数据/场景/文档)
③ 请对方评估: "看这样解释您还有疑问吗?"
示范:
评论 5: "线程安全没考虑, 你这个 map 是 HashMap"
张伟回复:
"谢谢老李, 关于线程安全我要澄清:
这个 map (channelMap) 在构造函数中初始化后即为只读,
参考 line 32 的 `Collections.unmodifiableMap`.
在只读场景下 HashMap 是安全的。
如果我漏考虑了什么并发场景, 请具体指出? 我马上验证。"
关键动作:回击不是对抗, 是"帮 Reviewer 理解上下文"。Reviewer 会感激,而不是记仇。
# 4.2 让步升级
疑惑:什么时候不该硬顶,该让步?
论证:让步的三种情形:
① 评论技术上对
评论: "SQL 注入风险"
现实: 确实拼了字符串
→ 立刻改, 感谢指出
② 评论虽然有争议, 但影响不大
评论: "变量名 c 改成 channel"
现实: 都能接受
→ 直接改, 别争 nit
③ Reviewer 是权威且不改也没大问题
评论: "建议用 Builder 模式"
现实: 你想省事, 但 Reviewer 是老王
→ 让步, 未来 5 分钟改完的事不值得较劲
升级的两种情形:
① 评论方法论层面, 涉及团队规范
评论: "package 结构应该按 DDD"
现实: 这不是你一个 PR 能决定的, 涉及整个团队
→ 升级到 Leader / 架构组统一决策
话术: "package 结构涉及全项目, 建议起一个专门议题
在下周架构对齐会上讨论, 本 PR 沿用现有约定"
② 评论超过你能承担的返工
评论: "综上建议整体推翻"
现实: 已经写了 3 天, 时间不允许
→ 升级到 Leader / 项目经理
话术: "整体重写会影响 M2 的交付,
建议本 PR 满足功能 + 关键指标,
优化项拆到下个迭代, 已录 Ticket #456"
结论:让步、回击、升级 三选一——根据评论级别 + 时间 + 权威度决定。
# 4.3 存档与外部背书
疑惑:面对攻击型评论怎么办?
论证:攻击型评论要处理,但不能激化。三步走:
Step 1: 就事论事回复, 语气平静
Step 2: 私下找 Leader 反映, 存档
Step 3: 请第三方架构师背书
示范 - 面对"你这写法有点像刚入行":
公开回复 (专业):
"感谢您指出, 想具体请教一下:
'刚入行的写法'具体是哪个点?
是命名、结构、还是逻辑?
我想理解并改进。"
私下 (Leader 反馈):
"老王, 这条评论 (贴链接) 我觉得越界了。
具体的技术问题都欢迎, 但我希望后续避免这种表述。
我已经在 PR 里做了专业回复, 想跟您对齐处理原则。"
外部背书 (架构师):
"老陈, 我这个方案下周想请您独立评审一下,
主要是这段 (贴链接), 想听听您的看法。"
关键:攻击型评论 → 你越平静, Reviewer 越显失态。观众判定站在你这边。
结论:Code Review 不是私斗——存档 + 上报 + 背书三管齐下,把关系博弈拉回到"技术评审"。
# 5. 评审别人的黄金句式
# 5.1 措辞的杀伤力
疑惑:反过来,我 Review 别人时怎么措辞才不伤人?
论证:同一件事,措辞不同,效果差 10 倍:
❌ 断言式:
"这里错了"
❌ 教导式:
"你应该这样写"
❌ 人身式:
"你怎么这么写?"
❌ 讽刺式:
"这写法真是......"
✅ 请教式:
"这里我有个疑问, 场景 X 下会不会出问题?"
✅ 共谋式:
"如果这样改, 是不是能避免 Y?"
✅ 数据式:
"我们的规范 P.12 里推荐用 Z, 这里是不是也用一下?"
心理原理:代码是作者的孩子。你怎么说他的孩子, 决定他怎么回应你。
# 5.2 十条黄金句式
收藏这 10 条,直接用在你的评论里:
1. "有个疑问, 场景 X 下会怎么处理?" (Question 型)
2. "建议考虑改用 X, 因为 Y。可选。" (Suggestion 型)
3. "nit: 变量名建议改成 X" (明确标 nit, 表明不强制)
4. "这里必须改, 因为涉及 X (安全/合规)。" (Blocker 型, 说清原因)
5. "我在 X 场景下遇过类似坑, 供参考: <链接>" (给经验)
6. "跟我们规范 P.X 不一致, 是有意为之还是可以对齐?" (给规范)
7. "如果是我我会 X, 但你的做法也 OK, 你判断。" (给选择权)
8. "这段和 X 类似, 是否可以复用? 或者独立不复用是有原因?" (探询)
9. "整体思路很好, 只有 3 个 nit 请看下。" (先肯定后细节)
10. "赞 X 的做法, 值得团队推广。" (显性认可)
结论:这 10 条覆盖 95% 场景——记住 3 个也够用。
# 5.3 提问代替断言
核心技巧:尽量把断言改成提问。
断言 → 提问
──────────────────────────────────────────
"这里错了" → "这里我不太确定, 能否解释一下?"
"用 Redis" → "有没有考虑过 Redis?"
"应该拆分" → "拆成 3 个是否更清晰?"
"命名不对" → "这个名字含义是什么? 我担心 XX 场景会误解"
"不应该复制" → "这段和 XX 类似, 有无复用可能?"
心理原理:
断言 → 作者感觉"被评判" → 防御 → 冲突
提问 → 作者感觉"被咨询" → 思考 → 合作
结论:Review 的目的是让代码更好, 不是让作者认错。提问比断言效果好 3 倍。
# 6. Review 分层策略
# 6.1 三层评审模型
疑惑:所有 PR 都应该被所有人 review 吗?
论证:Review 分三层, 深度不同:
┌────────────────────────────────────────────────────────┐
│ 三层评审模型 │
├────────────────────────────────────────────────────────┤
│ │
│ Layer 1: 功能正确性 (Correctness) │
│ - 逻辑对不对 │
│ - 边界条件 │
│ - 测试覆盖 │
│ → 主评审 (最熟业务的人) │
│ │
│ Layer 2: 架构一致性 (Architecture) │
│ - 分层是否合规 │
│ - 是否引入循环依赖 │
│ - 命名/包/接口设计是否合规 │
│ → 副评审 (Tech Lead / 架构师) │
│ │
│ Layer 3: 风格与优化 (Style & Polish) │
│ - 命名、格式、注释 │
│ - 微优化 │
│ → 建议评审 (任何有兴趣的团队成员) │
│ │
└────────────────────────────────────────────────────────┘
# 6.2 层次决定深度
关键规则:
Layer 1 主评审:
必须至少 1 位, 有 approve 权
必须逐行看核心逻辑
预期时间: 30-60 分钟
Layer 2 副评审:
必须至少 1 位, 有架构否决权
重点看架构一致性
预期时间: 15-30 分钟
Layer 3 建议评审:
可选, 无 merge 阻断权
只提 nit 和 suggestion
预期时间: 5-15 分钟
对照张伟的 17 条评论:
5 位架构师参与 = 全是 Layer 2/3
Layer 1 的主评审老王反而没时间深看
→ 结果: 张伟被 Layer 2/3 的意见淹没
Layer 1 的核心正确性反而没充分讨论
结论:分层错位 = 大 PR 灾难。规范里应该定义"每个 PR 必须有 1 位 Layer 1 主评审"。
# 6.3 谁 Review 什么
推荐规则:
| PR 类型 | Layer 1 主评审 | Layer 2 副评审 | Layer 3 建议 |
|---|---|---|---|
| 业务功能 | 同组资深 | Tech Lead | 任意 |
| 架构改造 | Tech Lead | 架构组 | Leader |
| 核心中间件 | 该中间件 owner | 架构组 + 安全 | 任意 |
| 数据库 schema | DBA | Tech Lead | 任意 |
| 前端 UI | UI 设计师 + 前端 Lead | / | 任意 |
关键:PR 描述里明确写出主评审是谁——避免 5 位都以为自己是主评审的悲剧。
# 7. 大 PR 的评审技巧
# 7.1 一次能读多少
疑惑:300 行的 PR 到底能不能一次评审?
论证:行业数据:
Cisco 研究 (2006):
- 200 行以下: 缺陷发现率 70%+
- 200-400 行: 缺陷发现率 40%
- 400-800 行: 缺陷发现率 25%
- 800+ 行: 缺陷发现率 < 10%
结论: 每 200 行是一个 "评审注意力单元"
结论:300 行 PR 就已经在"低效评审区"——Reviewer 只能挑到表面问题, 深层 bug 溜过去。
规则:
✅ ≤ 200 行: 一次评审
✅ 200-400 行: 强烈建议拆分
❌ > 400 行: 必须拆分, 除非有特殊理由
# 7.2 分段评审法
疑惑:拆不了怎么办?
论证:拆不了就分段评审:
张伟 300 行 PR 分段建议:
第 1 段 (Line 1-60): 接口定义 + 数据模型
评审时间: 15 分钟
重点: 契约设计
第 2 段 (Line 61-150): 核心算法
评审时间: 30 分钟
重点: 逻辑正确性
第 3 段 (Line 151-250): 缓存/优化
评审时间: 15 分钟
重点: 性能
第 4 段 (Line 251-300): 测试
评审时间: 10 分钟
重点: 覆盖率
Reviewer 侧的分段技巧:一次评审只看一段, 分 4 次评审完。注意力恢复后, 缺陷发现率维持在 60%。
# 7.3 走查会议辅助
疑惑:太复杂的 PR 光看 diff 不够怎么办?
论证:走查会议 (Code Walkthrough)——作者屏幕分享, 主评审面对面看。
适用场景:
- PR > 300 行
- 涉及新架构 / 新中间件
- 有 3+ 位关键 Reviewer
流程 (45 分钟):
- 0-5 min: 作者讲背景 + 变更目标
- 5-25 min: 逐段过代码, 主评审提问
- 25-40 min: 副评审提问
- 40-45 min: 达成共识 + 下一步 Action
产出:
- 会议纪要列出必改项 (Blocker)
- 建议项作者自行判断
- 会后 24h 内作者更新 PR
结论:走查会议 = 大 PR 的救命稻草。比 17 条书面评论有效 5 倍。
# 8. Review 反模式
# 8.1 只挑不给方案
症状:"这里不对" / "这样不好"——没说怎么改。
危害:作者不知道怎么办, 只能来回问, 效率极低。
修复:每条评论必须包含"改进方向"——即使是 "建议试试 X" 也比 "这里不对" 好 10 倍。
# 8.2 无关个人偏好
症状:"我不喜欢 Java Streams, 你别用" / "我觉得应该都用 Optional"。
危害:把私人品味当团队规范——作者无所适从。
修复:只针对团队规范和技术必要性提意见——个人偏好不该出现在 Review 里。
# 8.3 教条主义评审
症状:"必须用策略模式" / "SOLID 违反了"。
危害:为了模式而模式——过度设计,增加复杂度。
修复:"这个改动能带来什么具体收益" — 说不出具体收益的模式改造,不该强推。
# 8.4 迟评审黑洞
症状:PR 提了 3 天没人 review。
危害:作者上下文丢失,冷启动成本高。分支越搁越难 rebase。
修复:团队 SLA——24 小时内至少 1 次响应, 48 小时内主评审给出定性。
# 8.5 情绪化对抗
症状:作者:"我改的都对!" / Reviewer:"你懂什么!"
危害:技术讨论变私斗——观众取关,项目瘫痪。
修复:Leader 介入, 把 PR 移到走查会议。能公开讨论的,不该在 PR 评论里对撕。
# 8.6 一次否决全部
症状:"综上, 建议整体推翻重写"。
危害:极度打击作者——3 天写的代码被一句话否决。
修复:否决整体前,必须 Leader/架构组一起决策。单个 Reviewer 无权一票否决。
# 9. 把 Review 变教学场
# 9.1 每次评审留一课
疑惑:Review 除了改 bug,还能怎么产出价值?
论证:每次 Review 都是团队知识流转的机会:
方法: 每次 Review 结束后, 作者 / 主评审 抽 5 分钟总结
产出模板:
┌────────────────────────────────────────────┐
│ PR #123 复盘 (存到 Wiki) │
├────────────────────────────────────────────┤
│ 变更: 支付路由算法 │
│ 教学点: │
│ 1. 只读 Map 用 unmodifiableMap 包装 │
│ 2. SQL 必须用 PreparedStatement │
│ 3. 大 PR 应该拆分成 3-4 个 │
│ 团队规范更新: │
│ - 已提 PR #456 更新规范 P.15 │
└────────────────────────────────────────────┘
结论:Review 教学化 = 团队复利——同样的 bug 不会在下一个 PR 重犯。
# 9.2 团队评审规范
建议团队制定 Review 规范(示例):
团队 Code Review 规范 V1.0
## 1. PR 大小
- ≤ 200 行: 常规评审
- 200-400 行: 需要拆分说明
- > 400 行: 走查会议
## 2. 评审角色
- 主评审: 1 位 (功能+架构 approve 权)
- 副评审: 1-2 位 (架构一致性)
- 建议评审: 任意 (nit 与 suggestion)
## 3. SLA
- 24h 内至少 1 次响应
- 48h 内主评审给出定性
- 作者 24h 内响应所有评论
## 4. 评论格式
- nit: xxx (可改可不改)
- Q: xxx (疑问)
- Suggestion: xxx (建议)
- Blocker: xxx (必改)
## 5. 冲突升级
- 24h 内 Reviewer/作者未达成一致 → Tech Lead 仲裁
- 涉及架构原则 → 架构组仲裁
# 9.3 Review 数据度量
度量指标(月度):
① 首次响应时间 (First Review Time)
目标: < 4h 工作时间
② PR 平均评论轮次
目标: < 3 轮
③ Merge 前平均时长
目标: < 24h
④ 大 PR 比例 (> 400 行)
目标: < 10%
⑤ 复审率 (Reopen 数)
目标: 上线后因 Review 漏过而回滚的 PR < 1%
结论:度量 → 复盘 → 改进 → 循环。没度量的 Review 规范 = 一纸空文。
# 10. 综合案例串讲
# 10.1 张伟的第 18 次评论
回到张伟。他跟老王聊完后, 静下心把 17 条评论按五级分类:
分类结果:
Blocker (4 条 - 必改):
#5 线程安全澄清
#11 Log 级别改 warn
#12 SQL 注入 → PreparedStatement
#6 测试覆盖率补到 80%
Suggestion (5 条 - 选择性接受):
#1 策略模式 → 现阶段 if-else 更清晰, 拒绝
#3 Guava Cache → 采纳
#9 Optional → 热路径, 拒绝
#10 Builder → 采纳
#15 DDD package → 升级到架构组
Nit (5 条 - 全改):
#2, #7, #8, #13 → 全部 15 分钟改完
Question (2 条 - 认真回答):
#4 和 3 年前旧代码重合 → 解释不复用原因
#16 业务场景定义 → 附上 PRD 链接
Attack (1 条 - 就事回击):
#14 "刚入行的写法" → 请教具体点
否决 (1 条 - 升级):
#17 整体推翻 → 升级到老王仲裁
然后他开始逐条回复,他把这个作为第 18 条自评论放在 PR 顶部:
[作者说明] 感谢 5 位架构师提出的 17 条评论。
我按以下策略处理, 请指教:
## 必改 (Blocker) - 已修复
- #5 线程安全: 澄清 channelMap 只读, 已加 unmodifiableMap
- #11 Log 级别: warn ✅
- #12 SQL: PreparedStatement ✅
- #6 测试: 覆盖率提到 82% ✅
## 采纳 (Suggestion)
- #3 Guava Cache: 已引入
- #10 Builder: 已重构 PayContext
## 说明后暂不改 (Suggestion)
- #1 策略模式: 当前 3 个分支, if-else 更易读,
拆分留待第 5 个分支加入时
- #9 Optional: 热路径每秒 3 万次, 有性能开销.
已 benchmark 附在 P.5
## 升级仲裁
- #15 DDD package: 涉及全项目, 已升级到架构组 (@老陈)
- #17 整体推翻: 已请 Leader (@老王) 仲裁
## 认真回答
- #4 3 年前旧代码不复用: 因业务逻辑已迭代, 复用反而增耦合.
详见 PR 描述附录 A.
- #16 业务场景: PRD 链接 <url>, section 3.2
## 就事请教
- #14 "刚入行的写法": 想请具体指出是命名、结构、还是逻辑?
愿意听并改进.
Nit 部分已全部修改 (#2, #7, #8, #13).
请各位再看一轮, 提前感谢! 目标下周三前 merge.
# 10.2 一次成熟评审全貌
24 小时后, 结果:
Reviewer A (架构师):
- #1 策略模式: "OK, 按你的判断"
- #15 DDD package: "已跟架构组对齐, 本 PR 沿用"
Reviewer B:
- #3 Guava: "赞"
- #4 3年旧代码: "理解, 通过"
Reviewer C:
- #5 线程安全: "unmodifiableMap 我漏看了, 我的问题"
- #6 覆盖率: "82% 达标, 通过"
Reviewer D:
- #14 (原作者): "抱歉之前措辞不当, 具体是 line 78 的 if
嵌套太深, 建议提早 return. 已提新评论 #19"
Reviewer E:
- #9 Optional: "看到 benchmark, 说服了"
- #10 Builder: "赞"
Leader 老王:
- #17 整体推翻: "本 PR 保持不动, 未来 refactor 单独立项"
48 小时后:PR merged。所有 Reviewer 认可,老王事后跟张伟说:
老王: "这次你处理得漂亮。三件事我特别欣赏:
1. 分类回复, 没被淹死
2. 部分坚持, 用数据说话
3. 引架构组仲裁 DDD package, 是资深动作"
张伟: "我明白了, Code Review 不是挨批, 是博弈。"
# 10.3 评审哲学回扣
回望张伟这次翻盘,代码审查攻防能力的本质是:
初级: 全部接受 (被淹)
中级: 部分接受 (会挑刺)
高级: 分级应对 (Nit/Q/Sug/Blocker/Attack)
资深: 引导 Review (指定主评审 + 分段 + 走查)
大师: Review 教学化 (每次留一课, 团队复利)
Review 应对的三层修炼:
第一层: 分类能力 (17 条不再淹死你)
第二层: 应对策略 (回击/让步/升级 三选一)
第三层: 主导 Review (PR 描述指定主评审 + 分段)
核心命题:Code Review 是团队日更的战场——你处理 Review 的方式,累积决定你在团队中的位置。
结论:能优雅处理 17 条评论的人 = 能扛住 100 人团队 — 这是资深工程师的重要能力信号。
# 10.4 评审话术速查
Code Review 应对速查表:
| 场景 | 动作 |
|---|---|
| 收到 Blocker | 立刻改 + 感谢指出 |
| 收到 Suggestion | 判断价值/时间, 采纳或拒绝(附理由) |
| 收到 Question | 认真回答 + 展示思考深度 |
| 收到 Nit | 全接受, 5 分钟改完 |
| 收到 Attack | 就事回复 + 存档 + 私下反馈 Leader |
| 收到"整体推翻" | 立刻升级 Leader/架构组仲裁 |
| PR 冲突升级 | 24h 未达一致 → Tech Lead 仲裁 |
| 大 PR 300+ 行 | 拆分 or 走查会议 |
| PR 描述必写 | 目的 + 主评审 + 时间线 + 关注点 |
| 作者自评论 | 顶部说明"如何处理 N 条评论" |
十条黄金句式(Review 别人时):
1. "有个疑问, 场景 X 会怎么处理?"
2. "建议改用 X, 因为 Y。可选。"
3. "nit: 变量名建议改成 X"
4. "这里必须改, 因为 X (安全)。"
5. "我在 X 场景遇过类似坑: <链接>"
6. "跟规范 P.X 不一致, 有意为之?"
7. "如果是我我会 X, 你的做法也 OK, 你判断。"
8. "这段和 X 类似, 可以复用吗?"
9. "整体很好, 有 3 个 nit 请看下。"
10. "赞 X 做法, 值得团队推广。"
PR 描述模板:
## 变更目的
XX 支付路由算法, 关闭 issue #123
## Review 期望
- 主评审: 老王 (功能+架构)
- 副评审: 老李 (性能)
- 建议评审: 老张 (风格)
## 时间紧迫度
下周三前必须上线
## 特别请关注
- 支付路由核心 (PayRouter.java L45-180)
- 缓存策略 (是否用 Guava)
下一集预告:Higress 网关方案落地过程中,张伟发现需要四个部门协作——业务/架构/运维/安全。第一次跨部门推进,老板下命令"两周搞定", 结果第 3 周他还在群里问"XX 什么时候能给数据",项目原地打转。**第 06 篇《做事闭环执行能力》**告诉他"拿结果不是交作业"。
本篇速查表:
| 维度 | 关键武器 |
|---|---|
| 分类 | Nit/Q/Sug/Blocker/Attack 五级 |
| 应对 | 就事回击 / 让步 / 升级 三选一 |
| 评审措辞 | 提问代替断言 + 十条黄金句式 |
| 分层 | Layer 1/2/3 + 主/副/建议评审 |
| 大 PR | ≤ 200 行 / 分段 / 走查会议 |
| 反模式 | 只挑不给/私人偏好/教条/迟评/情绪/一票否决 |
| 教学化 | 每次留一课 + 团队规范 + 度量 |