真正提升代码质量的代码评审实践

大多数代码评审并不能提升代码质量。它们确认代码能编译，抓住一两个命名不一致，几分钟内就被批准了。我见过很多团队做了成百上千次这样的评审，然后疑惑为什么同类 bug 总是反复出现在生产环境中。能抓住真正问题的评审和走过场的评审之间的区别，不在于评审者的能力——而在于评审者手里有没有具体的东西可以对照。当一份写有验收标准的规格存在时，代码评审就变成了验证；当它不存在时，代码评审就只是观点的交换。

发布于 2025-12-21 · ✓ 已更新 2026-05-06 · 阅读约 8 分钟 · 作者：Daniel Marsh · 审校：编辑政策

为什么大多数代码评审抓不到真正的缺陷

回想一下你最近批准的十个 Pull Request。你有几次打开过对应的规格文档，甚至确认过规格是否存在？在我合作过的大多数团队里，答案是零。评审者打开 diff，扫一遍明显的问题，也许留一条关于变量命名的评论，然后点击批准。整个评审过程不到四分钟。

这不是偷懒，而是结构性问题。没有验收标准可供对照，评审者就没有客观依据说"这是错的"。他们能发现语法问题、风格违规和明显的逻辑错误，但他们无法验证代码是否处理了空购物车边界情况——因为没人记录过这个边界情况很重要。他们无法确认错误响应格式是否符合 API 契约——因为 PR 描述里没有链接到契约。

结果就是，评审流程大约只捕获了它能捕获的 15% 的缺陷。其余 85% 直接溜到 QA 或生产环境，修复成本是评审阶段的 5 到 50 倍。

规格存在时评审会发生什么变化

当评审者可以同时打开一份带有可测试验收标准的规格和代码 diff 时，评审就从主观阅读转变为客观验证。评审者不再问"这段代码看起来合理吗？"而是问"这段代码满足第 4 条标准吗？"这是本质上不同的问题——一个产生是或否的答案，而不是一种感觉。

下面看看实际效果。假设一个功能是在结账流程中添加优惠券验证。没有规格时，评审者看到验证逻辑，确认它不会崩溃就算完了。有规格时，评审者会逐条检查：

• 当使用过期优惠券时，代码是否返回 422 和 {"error": "coupon_expired"}？（标准 3）
• 代码是否将折扣上限设为购物车小计，确保总金额永远不为负？（标准 5）
• 代码是否处理了创建购物车时优惠券有效、但结账时已过期的情况？（边界情况 2）
• 是否有针对并发场景的测试——两个请求同时使用同一张一次性优惠券？（边界情况 4）

没有规格，评审者可能根本不会想到检查第三和第四项。有了规格，这些检查只需几秒钟——打开测试文件，搜索用例，确认它存在并断言了正确结果。

五种扼杀评审效果的反模式

十二年来，我反复看到同样五种模式将代码评审变成合规仪式，而非质量关卡。如果你的团队存在其中任何一种，评审正在制造虚假的安全感。

1. 橡皮图章式审批。评审者扫一眼 diff，两分钟内就批准。当团队存在隐性的社交压力不想阻塞队友、或者评审者足够信任作者而跳过审查时，这种情况最常发生。信任不是缺陷检测机制。

2. 纯风格评审。所有评论都是关于命名、格式或 import 顺序。这些是 diff 中最容易看到的，也是最不可能导致生产事故的。如果你的评审评论 90% 是风格问题，那你的评审 90% 是浪费。用 linter 自动化风格检查，把评审时间花在行为上。

3. 架构空想式评审。评审者在评论中重写整个 PR。"我会在这里用策略模式。""这应该是事件驱动管道。"这些评论与代码是否正确无关——它们只是评审者偏好不同的设计。除非当前设计违反了规格或造成了可度量的问题，否则这只是拖延交付而不提升质量的噪音。

4. 无上下文的 PR。PR 描述写着"修复 bug"或"添加功能"，没有链接到任何规格、工单或验收标准。评审者只能猜测代码应该做什么。即使是最认真的评审者也无法验证他们不知道的行为。

5. 一次评审太多代码。2000 行的 PR 和 200 行的 PR 获得同样质量的评审：都是浮于表面。研究一致表明，超过 200-400 行代码后，缺陷检测率急剧下降。如果 PR 更大，评审不是按比例变差——而是本质上变差。要么拆分，要么接受评审只是走形式。

基于规格的评审清单

这是我给采用了规格先行开发的团队使用的清单。它假定存在一份带有验收标准的规格。如果不存在，清单的第一项就是"要求提供规格"。

打开 diff 之前：
[ ] 阅读规格的验收标准
[ ] 阅读规格的边界情况部分
[ ] 记录已定义的错误路径

阅读 diff 时：
[ ] 每条验收标准——是否有对应的实现代码？
[ ] 每条验收标准——是否有验证它的测试？
[ ] 每个已定义的边界情况——是否被处理？
[ ] 每条错误路径——代码行为是否与规格一致？
[ ] 是否有不在任何标准范围内的代码路径？（范围蔓延）

阅读 diff 之后：
[ ] 在本地运行测试或确认 CI 通过
[ ] 确认 PR 没有修改规格范围外的行为
[ ] 评论引用具体标准，而非个人偏好

第三类检查——不被任何标准覆盖的代码路径——是大多数团队会跳过的。它能捕获范围蔓延：代码做了规格从未要求的事情。这些代码没有经过验收标准评审、边界情况分析或 QA 规划，是 PR 中风险最高的代码。在没有规格的评审中，它永远不会被质疑。

有规格评审与无规格评审的对比

为了使差异更加具体，下面列出评审者在两种模式下能够和不能够验证的内容。这不是理论推演——而是基于对三个团队八个月评审结果的跟踪。

表格后四行才是真正的价值所在。它们是导致生产事故的类别，而在无规格评审中基本上是不可见的。

如何给出能改变结果的反馈

即使有规格，大多数评审评论产生的是争论而不是改进。问题通常出在反馈的表述方式。比较以下两条对同一段代码的评论：

评论 A（基于观点）：
"我觉得这里应该处理 null 的情况。"

评论 B（引用规格）：
"标准 3 规定：'给定一个没有保存地址的用户，
API 返回 200 和一个空数组。'这段代码在
addresses 为 null 时会抛出 NullPointerException。
见第 142 行——addresses.stream() 没有 null 检查。"

评论 A 引发辩论。作者可以回复"我不认为这里会出现 null"，然后你们就陷入关于运行时假设的拉锯。评论 B 是一份缺陷报告。它指名了标准、确定了差距、指向了确切的行。没有什么可辩论的——规格要么这么写了，要么没有；代码要么处理了，要么没有。

让评审评论具有可操作性的三个原则：

• 引用标准。"这不符合标准 5"比"我觉得这不对"有用得多。它将对话从观点转向事实。
• 区分阻塞和非阻塞。将评论标记为"必须修复"（规格违反、缺失边界情况）或"小建议"（风格偏好、小重构）。如果所有评论都不加标记，作者会把所有东西都当成可选的。
• 描述可观察到的后果。"如果用户在 500 毫秒内提交两次表单，两个请求都会成功并创建重复订单"是一份 bug 报告。"这可能有竞态条件"是一种担忧。Bug 报告会被修复，担忧会被忽略。

在评审中发现和处理边界情况

规格应该在编码开始前就定义好边界情况，但评审者在代码评审中仍然会发现新的边界情况。这是预期中的——阅读实现代码会揭示规格作者未曾预料到的状态和转换。关键在于正确处理这些发现。

当你在评审中发现新的边界情况时，不要只是留一条评论说"X 怎么办？"而是应该：

• 用 Given/When/Then 格式描述这个边界情况，与规格定义边界情况的方式一致。
• 要求作者将其加入规格（如果是真正的需求），或记录为什么它是非目标。
• 如果被加入规格，PR 在获得批准前应包含对应的测试。

这样就形成了闭环。边界情况成为永久记录的一部分——而不是埋在一个没人会再看的 PR 评论里。下一个处理这段代码的人会在规格中看到这个边界情况，在测试套件中看到对应测试，而不是在六个月前的评审讨论中。

坚持这样做的团队会发现他们的规格随时间越来越好。每次评审周期都将边界情况反馈到规格中，同一领域的后续功能从第一天就拥有更好的覆盖率。

如何在团队中推行

流程变更如果以命令形式引入而缺乏上下文，注定失败。"所有 PR 必须链接到规格"能获得合规，但得不到真正的采纳。以下是在我经历的四个不同团队中都行之有效的推进步骤：

第 1-2 周：选择一个功能。选一个即将开始实现的功能。写一份带有可测试验收标准的规格。使用上面的清单对着规格进行代码评审。记录下这次评审抓到了哪些普通评审不会发现的问题。

第 3-4 周：分享结果。在团队回顾或工程会议上，展示规格驱动评审发现的具体缺陷。不是理论——是实际的 bug。"这次评审发现了地址字段缺少 null 检查，该问题本会导致 12% 的用户遇到 500 错误"，这种说法的说服力远超"规格有助于评审"。

第 5-8 周：扩展到愿意参与的人。不要在所有 PR 上强制推行。让看到结果的工程师在下一个功能上尝试。给他们清单。一个月后，你就会有足够的数据来证明它是否有效。

第 3 个月以后：设为默认流程。到这个时候，团队已经看到了足够多的具体案例，流程自己就能推销自己。PR 模板包含规格链接字段，评审清单已被收藏。跳过规格的工程师能感受到差异，因为他们的评审明显不那么有用了。

在这件事上失败的团队试图在一个冲刺内从零到全面执行。成功的团队让结果自然产生牵引力。