软件团队代码审查要点 » Blake Smith
来源 (中文翻译版): https://blakesmith.me/2015/02/09/code-review-essentials-for-software-teams.html 抓取时间: 2026-07-21 16:27:51
Blake Smith\n## create. code. learn.\n 关于\n 文章\n 项目\n 演讲\n github
»
软件团队代码审查要点
2015年2月9日 代码审查是任何协作软件项目的重要组成部分。大型软件系统通常由多人编写，因此高效运作的软件团队需要一个强大的流程来保持团队成员以及代码库本身朝着正确的方向前进。 代码审查是一个强大的工具，它能够：
- 帮助团队成员在系统变化时调整他们对系统的心智模型
- 确保变更正确解决了问题
- 开启关于设计优缺点的讨论
- 在 bug 进入生产环境之前捕获它们
- 保持代码风格和组织一致性
将这些好处视为需求层次会很有帮助。
保持团队成员团结一致
代码审查最关键的功能是保持团队的每个成员朝着正确的方向前进。你无法安全地改变一个你不理解的系统，因此代码审查保持团队的心智对齐。当 Bob 提交会计子系统的 pull request 时，Amy 在审查 Bob 的代码时更新了她对该系统的心智模型。Amy 有机会就她不理解的部分提问，Bob 也受益于澄清他的设计决策，同时也向其他人传授了他的工作。当 Amy 一个月后必须对会计子系统进行更改时，她对系统的心智模型已经是最新的，可以立即采取行动。她花更少的时间阅读代码并尝试在大脑中拼凑系统，而花更多的时间思考更高层次的抽象和设计。每个人都赢了，因为每个人都保持在一起。
执行一个好的 Pull Request
在你编写任何一行更改系统的代码之前，问问自己以下问题：
- "这是正确的工作方向吗？" 客户、内部团队成员和其他各方总会有竞争的需求。在深入研究之前，这是保持优先级清晰的好方法。其他流程如迭代/冲刺规划会议也有助于保持这一点。
- "团队是否已经同意这个变更是正确的？" 如果没有，最好通过电子邮件或面对面开始设计讨论。当人们在你更改之前就设计达成一致时，你的变更更有可能被接受。
- "我如何将这个变更分解为易于审查和理解的小块？" 小变更更容易思考和理解。良好的讨论源于团队能够快速理解你的变更。如果你的变更非常庞大，队友的眼睛会呆滞，你可能只会从他们那里得到一些风格上的挑剔。
- "我将如何测试这个变更以消除 bug 并确保正确性？" 你可能有一个 QA 部门，但作为开发人员，交付高质量的工作软件仍然是你的工作。易于测试的软件通常更加解耦，被分解成更小的块，并且更容易推理。你需要为所有变更制定测试计划。
我发现，从长远来看，提前回答这些问题为我省去了很多麻烦。你最不想做的事情就是花几天时间编写一个变更，结果却因为基本的设计缺陷或团队分歧而被拒绝。或者因为没有人能够验证其正确性而导致你的变更被搁置。再次强调，目标是改变系统，同时保持其他团队成员对你的变更了解。问问自己如何将工作分解为小块并测试这些块在很多方面都有帮助：
- 它降低了风险
- 它使变更更容易推理
- 它推动你走向更好的设计
你宁愿用手术刀做许多小而精确的切割，也不愿用大砍刀做一个巨大的伤口。在大多数增量情况下，我更喜欢手术刀驱动的开发，并且喜欢在需要删除大块死代码时才使用大砍刀。
发送 Pull Request
好的，你已经获得了团队对变更的认可，并且你已经实现了想要构建的设计。实际上发送 pull request 最有效的方式是什么？你一直在努力进行变更，希望团队的其他成员关注你的 pull request 并给你快速的反馈。你怎么能做到这一点？ 还记得我之前说过代码审查最重要的部分是保持团队的集体心智模型良好对齐吗？你的其他队友可能正在做与你完全不同的事情，可能在系统的完全不同的部分。他们的大脑处于完全不同的上下文中，所以你必须通过给他们进行审查所需的有用指导来克服这一点。这意味着写一个组织良好的描述，说明你做了什么变更，为什么做这些变更，以及他们仅通过阅读代码无法获得的任何其他相关信息。不要让你的队友做不必要的脑力劳动。 让我们看看一些好的和坏的 pull request 描述示例： 糟糕的示例：
标题：修复未初始化内存 bug
描述：
这是我和 Bob 之前讨论过的 bug。我在编译器方面遇到了
一些麻烦，但设法让它工作了。让我知道你们的想法。
如果你是这个 pull request 的开发者，停下来设身处地为代码审查者着想。标题模糊不清，提出的问题多于答案：内存 bug 在哪里？这个变更有多关键？你和 Bob 之前讨论的 bug 是什么？你在编译器方面遇到了什么麻烦？描述没有提供关于问题的任何有用上下文，也没有提供对你的变更的任何有用描述。如果这个 pull request 很长，审查者将不得不深入代码并进行脑力体操，试图获得上下文，然后才有机会思考这个变更如何适合系统的连贯设计。 这是一个改进的示例，有助于阐明变更集：
标题：修复从未初始化内存导致的启动时进程崩溃 [#54633]
描述：
由于我们的统计 Counter 类中的内存初始化错误，
这个 bug 导致启动时进程崩溃。我和 Bob 讨论了
这个问题，我们都同意崩溃是一个罕见的边缘情况，
不需要进行热修复发布。以下是变更摘要：
- 将底层 int 变量移到类初始化器中
以防止 Counter 中的未初始化内存。
- 重新设计了 Counter 接口以简化调用者的条件
逻辑并防止进一步的差一计数问题。
- 添加了一个暴露崩溃的单元测试
测试：我已经验证测试套件仍然通过，并且手动
验证了本地不会发生崩溃。
这个 pull request 描述改进了几点：标题足够描述性，给审查者一些简短的上下文，并吸引他们点击电子邮件了解更多。审查者知道这是一个进程崩溃（通常是非常糟糕的事情），还有一个 bug 报告编号，如果审查者有兴趣了解更多关于 bug 报告的信息，可以在那里阅读更多详细信息。新描述还概述了问题发生在哪里，并帮助提供了关于修复有多关键的一些上下文。有一个摘要列出了 pull request 中的高级结构变更，这将在审查者查看代码之前给他们一个变更的心智图景。代码审查者不应该对他们发现的东西感到惊讶。这个示例还概述了执行的测试，这将让审查者相信变更经过了深思熟虑、良好测试并准备好合并。让你的审查者容易成功，让他们点击"合并"变得极其简单。
审查者：给予建设性反馈
审查过程的下一部分是考虑如何向同行提供建设性反馈。如果目标是清晰和对齐，提供高质量的建设性反馈有助于每个人更好地理解系统并推动更好的代码。 避免说这样的话：
- "这个设计坏了。" 为什么坏了？如何改进？这样的陈述会伤害信心并挫伤自尊心。
- "我不喜欢这个变更。" 为什么不喜欢？你想要什么？不喜欢某件事是可以的，但你应该表达你的想法并提供关于如何改进代码的有用线索。
- "你能重写这个让它更清晰吗？" 我现在的代码有什么问题？我应该如何重写？什么不清楚？这样的评论本身就不清楚，没有提供简单的前进道路。
相反，说这样的话：
- "这段代码如何处理负整数？" 这个反馈是具体的，会让开发者自己思考结果。作为审查者，你可能知道有问题的代码会因负整数而崩溃，但让开发者自己理解这一点更好。这样的问题也可能表明存在测试差距，或者需要进一步规范范围。
- "这部分让我困惑，我不理解为什么 A 类与 B 类通信" 如果开发者没有提供有用的描述，并且代码结构不整齐，这种评论有助于推动设计的清晰性和更简洁的代码。
- "看起来你在这里打破了一个接口边界。这将如何影响用户？" 你已经指出了你注意到的问题，相信他们是有意这样做的。现在他们可以思考打破接口边界的看不见的后果，并提供他们决策的理由，或者选择改变它。
一般来说，将反馈框架为问题是推动清晰性和正确性的好方法，同时帮助开发者在未来改进他们的设计。这通常是高质量创意写作小组互相提供反馈的方式。在创意写作环境中，说"我不喜欢这个角色"这样的话是有害的，而同样的评论可以更清晰地重新表述："在第一章中，你的角色是热情和富有同情心的，现在他又冷又冰冷。他在我看来不像一个真实的人。" 现在有了可以澄清以找出问题的具体反馈。 程序员喜欢解决问题并指出问题，所以本质上他们喜欢发现和指出缺陷。很容易将代码审查视为通过在同行代码中发现问题来证明你有多聪明的方式。不要这样做。代码审查是让更多人关注变更并找出关键问题的一种方式，但你的目标应该是以鼓励团队成员在解决手头问题的同时提高技能的方式进行审查。
风格要点
括号位置、变量/函数名称、缩进和间距问题应该解决，但它们不是良好代码审查的核心目的（注意我把它们放在代码审查金字塔的顶部）。如果你发现你的团队花 90% 的时间挑剔缩进和变量名称，你可能正在把每个人的时间浪费在大多数可以自动化的事情上。编写一份风格指南，在签入时强制执行缩进和间距问题，然后花时间专注于更高价值的问题。我不想削弱一致风格的重要性。相反，拥有一致的惯用风格是让你的代码库易于阅读和理解的最简单方法之一。不过，如果你把所有的代码审查都集中在这些简单的任务上，问问自己是否在逃避更困难和更重要的工作：保持团队的心智对齐并思考更高层次的设计。 更多文章
关于作者
Blake Smith 是 Sprout Social 的首席软件工程师。