代码审查是一项技能

# 评审代码是一项技能 来源：https://typesanitizer.com/blog/code-review.html **目标读者：**对提升软件开发技能感兴趣的软件开发者。最初，我写这篇文章更多是面向初级开发者的，但有些部分也适用于技术负责人等角色。所以如果这篇文章看起来有点混杂，请接受我内心的帕斯卡的道歉：“抱歉，我没有时间把它拆成两篇文章。” 在开发者社区中，关于代码评审有很多讨论，尤其是在 2025-2026 年期间。例如，你可能见过类似这样的说法： 不一定都来自同一批人。 1. “代码评审是瓶颈” 2. “强制合并前评审是低信任环境的做法；你应该直接推送到主分支” 3. “代码评审发现不了 bug” 4. “代码评审不是为了 X，而是为了 Y” 5. “LLM 比人类更擅长评审代码” 6. “LLM 代码评审在发现边界情况和 bug 方面比人类强得多” 7. “你不应该再看代码了；你应该去做 XYZ 才对” 等等。其中，暂且聚焦一下“代码评审不是为了 X，而是为了 Y”，研究是这样说的： > 通过对访谈数据进行编码，我们确定了 Google 开发者对代码评审期望的四个关键主题：教育（education）、维护规范（maintaining norms）、把关（gatekeeping）和事故预防（accident prevention）。教育指的是在代码评审中教学或学习，这与引入代码评审的初衷一致；规范指的是组织对可自由选择事项（例如格式化或 API 使用模式）的偏好；把关涉及对源代码、设计选择或其他人工制品的边界的确立和维护；事故则指引入 bug、缺陷或其他与质量相关的问题。 > –《现代代码评审：Google 案例研究》(https://dl.acm.org/doi/pdf/10.1145/3183519.3183525)（2018 年） 类似地，《现代代码评审的期望、结果与挑战》(https://d1wqtxts1xzle7.cloudfront.net/123087856/icse2013-libre.pdf?1748877378=&response-content-disposition=inline%3B+filename%3DExpectations_outcomes_and_challenges_of.pdf&Expires=1786290544&Signature=R5O-pbRUD~ZBlCXj7MKPrQovHnP94AzTDdRaHH7OMLzSz-rElLhqbwYRJrN76q2ooGx6cRtl5laCF1fMf-rp3~CuR9vSOY2zWA20TuZCrQueownjjFMsMwMk16RgdsSgAPxtA~TXCOD06HqAo~wYeETeJW-l9gbRgbaFI2caM7pxbZM3eY2TvPQi54EAeymv3OyIylije4xUtlaoa9b7bwSfxSHwvxkaS2alYSqVu3HO1iZMTTQs9djOzEV3J8yGoJ26-PFhhwSUG1HIawGy6iGgOPur5Ue-EMWoLa85l5zGzh0jBdyJcf7VAaDIbU9wC4K~yXh9GzMocHpXV0YHDw__&Key-Pair-Id=APKAJLOHF5GGSLRBV4ZA)（2013 年）指出： > 我们的研究表明，尽管发现缺陷仍然是评审的主要动机，但评审与缺陷的关系比人们预期的要小，反而提供了额外的收益，例如知识传递、提高团队意识，以及产生解决问题的替代方案。此外，我们发现，理解代码和变更是代码评审的关键方面，开发者使用了各种机制来满足其理解需求，其中大部分是当前工具无法满足的。 所以至少，希望我们能够同意代码评审有多重目的。我稍后会谈到其他几点。但在此之前，我想阐明一个我在别处很少看到的框架，即**评审代码是一项技能**。具体来说，我提出： - 人是可以在评审代码方面变得更好的。这里的“更好”指的是上述所有目的：发现 bug、发现设计问题、提高对正在发生的事情的感知，以及理解代码。 - 可以教会一个人如何更好地评审代码。 - 由于这是一项相当现代的技能，我们并不完全知道人类技能的极限在哪里（例如，就速度与质量而言，帕累托前沿是什么样的？）。 - 如果你是一名软件开发者，并且你相信在可预见的未来，人类仍将参与程序的开发和维护，那么提升评审代码的能力就是**有价值的**。 首先，我将给出三个小例子，来自过去几周我在评审代码时发现 bug 的经历。我特意选择 bug 来讨论，因为它们相对没有歧义。接下来，我将提供一些我自己在代码评审方面的历史背景，以及支持核心论点的一些论据。之后，我会讨论一些关于试验和改进代码评审的想法。最后，我会讨论前面提到的那些经常被重复的关于代码评审的梗，以及它们在这个论点面前是否站得住脚。 让我们开始吧。 ## 三个差点被引入的 bug 的故事 在解释下面的不同例子时，我省略了很多细节，以使其更容易理解。留意那些“嗯，这看起来像是代码坏味道，难怪你差点出那个 bug”或者“啧，这本来可以用 XYZ 避免的”之类的想法，可能会很有价值。 三个案例中有两个，PR 的作者对相关代码有经验。另外要注意的是，下面描述的所有 PR 都运行了混合高端编码模型（大约 2026 年 6 月）的 LLM 评审。它们没有发现我发现的那些问题。 --- 参考 Lorin Hochstein 的《传统观点与韧性工程观点》(https://surfingcomplexity.blog/2026/08/02/traditional-versus-resilience-engineering-views/) 中的这张方便表格可能会有所帮助。这篇文章很短，推荐阅读。下面的表格是原文中表格的一个子集。 | 传统观点关注 | 韧性工程观点关注 | | --- | --- | | 目标 | 生产压力 | | 降低复杂性 | 驾驭复杂性 | | 根本原因 | 多种因素的相互作用 | | 人的可变性视为负担 | 人的可变性视为资产 | 阅读下面案例的一种方式是，试着在阅读时同时考虑表格的两侧。 ### 编写一些 git 配置 我们在 `$WORK` 使用我们自己的 devbox（用于软件开发的一次性虚拟机），它们运行在 EC2 实例上。启动逻辑由两个子进程组成： - 一个后台进程，用于初始化并非立即可用的状态。这个进程在用户开始使用 devbox 时可能已经完成，也可能尚未完成。 - 一个前台进程，它需要从用户的笔记本电脑获取一些额外数据，并阻塞用户直到完成。只有在这个进程完成后，用户才能开始使用 devbox。 为了减少延迟，我们一直在努力将更多操作移到后台进程。本着这种精神，我的一位同事创建了一个 PR，将全局 `~/.gitconfig` 的一些修改从前台进程移到后台进程。 为了性能和一致性，我们希望管理人们 Git 配置的某些方面。是的，我知道 Nix 存在，我自己的一台服务器就在用。不，我们工作时不使用 Nix。你在注意控制那些侵入性的想法，对吧？ 所以，当一个 `git config` 命令需要修改 `~/.gitconfig` 时，它首先获取 `~/.gitconfig.lock` 上的独占文件锁。这可以防止来自其他（协作）进程的并发修改，例如其他 `git config` 调用。 当我看到这个 PR 时，我想起了我们在 devbox 设置中曾经遇到过非确定性，git 在获取锁失败时快速失败的行为，加上类似的并发写入问题，导致了启动期间的随机失败。如果仅仅重试，仍然会导致非确定性，所以我的同事使用现有机制添加了一条跨进程依赖边，即只有在后台进程完成后才会进行写入。然后我指出，我们之前其实尝试过这个方案，但很快就把它去掉了，因为端到端延迟增加了（因为现在前台进程的一个子部分必须等待*整个*后台进程完成）。 最后，由于我们还有一些无法直接通过 `git config` 完成的文件修改（因为需要协调 `# DO NOT EDIT` 块），我们收敛到了一个解决方案，即使用独立的 `flock` 操作（带有自己的 `.lock` 文件）。这样既能（1）支持带退避的重试，（2）在同一个 `flock` 下进行多次修改而不会插入其他写入，（3）直接写入而无需担心并发写入者。 ### 显示还是不显示进度 有一个定期运行的 CI 任务，它进行一些处理并将 tarball 上传到 AWS S3 存储桶。结果发现，`aws` CLI 默认显示进度。想必这是为了帮助调试问题，以及在终端直接使用 CLI 时提供进度上的安心感。 当这位同事将上传逻辑从 1 个存储桶改为 4 个存储桶，以加快其他区域的下载速度时（AWS 存储桶属于特定区域），CI 任务开始失败，因为日志文件超过了 10MB 的限制。原来，`aws` CLI 每上传 256 KB 就会记录一行。当上传量达到 10GB+ 时，这对应数万行日志。日志行数乘以 4 后，任务就超过了 10MB 限制。 这位同事提交了一个 PR，将调用改为使用 `--no-progress` 来解除 CI 任务的故障。当时，我的第一个想法是：“嗯，这是唯一的选项吗？也许有一种方式可以显示更少的进度更新？如果任务在上传中途失败，最好有一些进度信息，以便更清楚地知道任务相对于上传开始是什么*时候*失败的。”于是我查了 `aws` CLI 的文档，看到了一个标志 `--progress-seconds <value>`，它可以调整进度更新的频率。我问 PR 作者能否改用这个。 在我脑海深处，我记得之前有过一个小事故：一个常用脚本引入了 `aws` CLI 标志，但脚本运行的一些环境中的 CLI 版本不够新，不支持该标志，导致脚本对很多人来说都坏了。由于 PR 作者也见过那件事，我假设他们会尽职尽责地检查 CI 任务中运行的 CLI 版本，并检查该 CLI 版本是否支持 `--progress-seconds`。 PR 作者很快回复了我，并更新了 PR，去掉了 `--no-progress`，改用 `--progress-seconds`。我对这个速度有点惊讶，所以我意识到他们可能没有考虑到同样的风险。首先，我尝试查看 aws CLI 的变更日志，看它是否提到该标志是什么时候引入的（因为该标志的文档中没有这一信息）。结果没有。然后我让 LLM 追踪 CI 任务使用的 CLI 版本，以及 `aws` CLI 中引入该标志的提交，以及包含该提交的第一个发布版本。这揭示了一个事实：如果使用 `--progress-seconds`，任务会挂掉，因为任务中现有的 CLI 版本太旧了。我对提交和版本做了一些抽查，然后在 PR 上评论，先简短道歉说没有早点说明我的假设，并询问他们能否交叉核验一下发现/寻找替代方案。 这个 PR 后来通过安装一个足够新的 `aws` CLI 版本修复了，而这个版本已经被打包在仓库中的另一个地方。 ### 校验和的作用是什么？ 还记得前面例子中的 CI 任务吗？我最近一直在做的事情之一，是将那个任务组的发布流程从整体 CI 发布流程中解耦出来。（CI 系统足够复杂，不同的任务可以有自己的发布流程。）有一天，在将我的 WIP 改动变基到 `master` 分支上时，我遇到了合并冲突。结果发现，有人添加了现有任务的新变体。 这个任务组负责上传某些 tarball。一位工程师需要再上传一些略有不同的 tarball。他们为此创建了一个单独的任务，而不是更新组内的现有任务。我当时想：“嗯，为什么这是一个不同的任务？不应该只是在某个列表里多加一个元素吗？”结果发现，那个添加任务 PR 的评审者建议先把新任务和旧任务分开，以降低干扰现有任务性能、可靠性等的风险。 此外，其中一个“细微”差异是，新任务在上传 sidecar 文件时附带校验和。（如果你读到这在想“但是 S3 不是已经原生支持校验和了吗”，其实我在这一切尘埃落定之后才知道这件事，我猜最初的 PR 作者也不知道。） 我看着这个感到很困惑。“如果这个任务需要校验和，那现有任务需要吗？如果需要，为什么不让校验和逻辑统一适用于所有任务？我们需要更新现有的读取方来检查旧 tarball 的完整性吗？这些新 tarball 是否跨越了某种信任边界，而旧 tarball 没有？” 再想了想，我意识到这里有一个 bug。大约一年前，我看过 Hannes Mühleisen 教授的一个演讲，叫《DuckLake – 面向其余所有人的 SQL 驱动 Lakehouse 格式》(https://youtu.be/YQEUkFWa69o?si=9om_lryDl57PBIno&t=919)。演讲谈到了 Apache Iceberg 及其对文件的使用，看起来非常复杂。（Apache Iceberg 的元数据布局：catalog 为每个表保存一个当前元数据指针，每个元数据文件跟踪快照，每个快照指向一个 manifest list，manifest list 指向 manifest 文件，manifest 文件最终指向数据文件。）当时我的理解是，这种复杂性的一部分原因是本质上没有办法进行多对象事务。因此，DuckLake 的一个关键设计决策是使用一个 SQL 数据库（支持 ACID 事务）来维护元数据。 回到 CI 任务，它把 tarball 上传到固定存储桶中的固定对象名。这个上传发生在校验和之前。所以，虽然 S3 保证对象写入的全有或全无语义，但如果任务在校验和上传之前被取消或崩溃，那么那些以故障关闭方式（校验和的通常用法）检查 tarball 与旧校验和完整性的读取方就会开始失败，这将导致该特定功能出现约等于宕机的情况，直到 tarball 和校验和重新同步为止。 我向创建该 PR 的人指出了这个故障模式，也在引入读取路径的 PR 中指出了。最终，读取路径对 sidecar 文件的检查没有被引入。 ## 为什么说代码评审是一项技能 让我们把时间稍微往回拨一点，到 2018 年。我当时是物理学研究生。我做的研究涉及各种模拟。我们很多工作使用 Jupyter notebook。模拟第一天能用，第二天就坏了，第三天，即使撤销了第二天的工作，仍然继续坏着。 我在努力解决如何管理代码、如何让它高效、如何防止错误出现，以及如何使用版本控制。那时，我开始学习自动化测试和代码评审。我记得当时非常惊讶：“等等，什么？人们真的会评审所有代码，而不只是输出吗？！哇，做软件开发者一定很棒。”后来，我对如何更好地编写软件的过程越来越感兴趣，对本来应该研究的物理学越来越失望。最终我在 2019 年退出了博士项目，开始做软件工程师。 --- 从那时到现在，在我迄今为止的职业生涯中，有多次有人评论说，他们觉得我的代码评审比他们通常接触到的平均评审更有帮助。举一个数据点：在 2026 年之前，在评审一个 PR 时，取决于作者对代码的熟悉程度，