2.5

View in English

2.5 代码评审与协作

概述与动机

代码评审是指在一项变更合并之前,由作者之外的人对其进行检查的实践。它是软件组织所拥有的杠杆效应最高的质量与知识共享活动之一,对大型团队而言,它也是协调与文化的一项主要机制。评审能捕获缺陷、传播对代码库的了解、强制执行标准、指导工程师成长()但前提是你做得好。做得不好,它就会变成瓶颈、摩擦的源头,或者一枚带来虚假安全感的橡皮图章。

对大型团队而言,评审是个人工作与集体所有权相遇的地方。它常常是原本各自独立工作的工程师之间主要的接触点,因此它的规范塑造着整个组织如何协作。评审传播知识,使系统的任何一个部分都不会只被一个人理解,从而降低了巴士因子风险()即知识集中在太少人手中的危险,这正是困扰大型、长生命周期系统的顽疾。它还会创建一条审计轨迹,记录谁改了什么、谁批准了它。

在企业和政府场景中,评审往往还带有合规维度。职责分离(没有任何一个人能单独掌控一项敏感变更的全部环节)、强制审批和可追溯性,通常都是被要求的控制项。一项触及敏感系统的变更可能需要由特定角色进行评审,而评审记录则会成为审计证据。你面临的挑战是,在满足这些控制要求的同时,让评审保持快速且富有建设性,而不是让它沦为一种形式主义。

关键原则

  • 评审是为了改进变更、传播知识,而不是为了炫技。
  • 小的变更能获得更好的评审,所以要让拉取请求(PR)聚焦、规模合理。
  • 评审延迟是一项团队范围内的成本。快速的响应能让所有人保持前进。
  • 把机械性的工作(风格、测试、安全扫描)自动化,让人来评审设计和正确性。
  • 把阻塞性问题与建议、偏好区分开来,并明确说明哪个是哪个。
  • 批评代码,而不是批评人。反馈规范决定了评审究竟是建立信任,还是侵蚀信任。
  • 让变更易于评审,是作者的责任。

建议

让拉取请求保持小巧且描述清晰

让每一项变更聚焦于单一的逻辑关注点,且规模足够小,便于仔细评审。庞大的 PR 只会得到浅尝辄止的评审。清楚描述改了什么、为什么改,以及你是如何验证它的,让评审者拥有充分的上下文。把机械性的重构和行为变更拆分到不同的 PR 中,让每一个都易于推理。一份好的描述,是作者对评审质量所能做出的最重要的单一贡献。

建立评审标准和检查清单

明确说明评审者应该关注什么:正确性、设计契合度、测试充分性、安全影响、可读性,以及对标准的遵循。一份轻量级的检查清单能让评审保持一致,防止重要维度被遗漏,同时又不会把评审变成勾选框的机械劳动。明确定义什么需要评审、谁可以批准,以及敏感领域所需的任何基于角色的审批。

设定并监控评审延迟规范

就目标响应时间达成一致,例如在一个工作日内做出回应,并把评审当作一天工作中的头等大事,而不是留到最后才挤出时间来做的事情。冗长的评审队列会拖慢交付,并诱使工程师提交超大的、批量的变更。监控首次评审耗时和合并耗时,把持续存在的延迟当作一个需要修复的流程问题,而不是个人的失职。

把一切机械性的事情自动化

在持续集成(CI)中运行格式化、代码检查、测试,以及安全和依赖扫描,让评审者永远不必为这些事情分心。把人工评审留给机器无法判断的事情:设计是否正确、方案是否契合系统、测试是否有意义,以及这段代码日后是否仍然讲得通。

在适合的场景使用结对编程和群组编程

对于复杂或高风险的工作、入职引导,以及知识传递,使用结对编程()两名工程师在同一工作站共同编写代码。这是一种持续进行的评审,往往能省去单独的评审步骤。对于关键的设计决策,或者为了在团队中传播对一个棘手领域的知识,使用群组编程()整个团队同时协作处理同一项任务。要把这些当作异步评审的补充,根据情境来选择,而不是要在所有地方强制推行的替代方案。

审慎采用自动化和 AI 辅助评审

使用自动化评审工具和 AI 助手来捕获常见问题、提出改进建议、减轻评审者的负担,但要把它们的输出当作输入,而不是权威结论。AI 评审擅长表层问题和一致性检查,而不擅长深层的设计判断和系统层面的语境理解。为每一次批准都保留一位负责的人,对安全敏感和合规相关的变更尤其如此。

设定建设性的反馈规范

设定规范,让反馈保持具体、友善,并聚焦于代码本身。鼓励评审者多提问而不是下命令,解释一项要求背后的理由,并对好的工作给予赞扬。清楚标记阻塞性关注点和可选建议(例如,为非阻塞性备注加上前缀)。这些规范决定了评审究竟是强化团队,还是滋生怨恨。

权衡:优点与缺点

方式优点缺点
异步 PR 评审灵活;有记录;能跨时区扩展存在延迟;容易丢失细微之处;可能感觉带有对抗性
结对编程持续评审;知识传递快;质量高两人共处理一项任务;令人疲惫;更难排期
群组编程全团队对齐;传播深层知识总体成本高昂;不适用于日常工作
强制多人评审保障力度强;对合规友好速度较慢;责任被分散;带来队列压力
AI 辅助评审对常见问题快速、不知疲倦;减轻负担缺乏系统语境;若过度信任会产生虚假的安心感

核心张力在于全面性与速度之间的取舍。更深入的评审能捕获更多问题,但会拖慢交付,也可能令作者感到沮丧。更快的评审能保持流动,但有流于表面的风险。破解之道是让评审深度与变更风险相匹配,使琐碎的变更获得轻量的评审,高风险的变更获得深入的评审,并把机械性工作自动化,好让人力集中在真正重要的地方。

与团队讨论的问题

  1. 什么样的规模才算「对一个拉取请求来说太大了」,你们会把机械性重构和行为变更拆分开吗? 本章明确指出,庞大的 PR 只会得到浅尝辄止的评审,可评审性由作者负责,并要求你们把重构和行为变更分开,以便各自都易于推理。在一个大型团队中,一个庞大的 PR 几乎必然导致橡皮图章式的批准,这带来虚假的安心感,却放过了真正的缺陷。带上证据:你们 PR 规模的分布情况,以及评审深度如何随着差异规模增大而下降。就一个切实可行的规模标准,以及把纯重构与逻辑变更分开提交的习惯达成一致,这样评审者才能真正把每一项变更装进脑子里。这一项单独的纪律,就能提升此后每一次评审的质量。

  2. 你们如何区分一项阻塞性反对意见和一项可选建议,这个约定是否真的被使用? 本章要求你们把阻塞性问题和偏好区分开来,并明确说明哪个是哪个,并指出以偏好为由阻塞是一种具有腐蚀性的反模式。没有共享的约定,评审者的风格偏好读起来就像是一项必需的变更,这会滋生怨恨,拖慢整个团队的交付速度。带上最近评审中因偏好而卡住合并的具体例子作为证据。采用一个轻量级的标记方式,例如给非阻塞性备注加上前缀,让作者能立刻知道哪些必须改,哪些只是建议。这能让评审保持聚焦于正确性和设计,而不是品味。

  3. 对于安全敏感或合规相关的代码变更,谁必须审批,这种路由是如何被强制执行的? 本章描述了基于角色的审批、代码所有权规则,以及职责分离()没有任何一个人能单独掌控一项敏感变更的全部环节,审批被记录为审计证据。在企业和政府场景中,这些是被要求的控制项,风险在于它们要么被跳过,要么变成一个冻结交付的瓶颈。带上证据:哪些模块是敏感的,目前的所有权规则是否能自动把这些变更路由给正确的审批人。把这种路由编码进代码所有权配置中,并搭配自动化检查和小规模变更,这样控制要求就能在没有人工把关队列的情况下得到满足。要有意识地做出这个决定,而不是在一次审计中才发现这个缺口。

  4. 你们实际商定的评审延迟目标是什么,你们是在度量并强制执行它,还是仅仅停留在美好愿望层面? 本章把评审延迟视为一项团队范围内的成本,要求你们监控首次评审耗时和合并耗时,把持续存在的延迟当作流程问题而不是个人失职。在一个大型团队中,一个无人负责的评审队列会悄悄向所有人征税:作者为避免等待而批量提交更大的变更,这些变更随后得到更浅层的评审,交付周期在没有单一元凶的情况下悄然拉长。与之对立的考量是,一个严苛的延迟目标可能会促使评审者草草浏览,所以速度和深度必须被平衡,而不能盲目取舍。带上证据:你们目前首次评审耗时的分布情况,它如何因团队和变更规模而异,以及评审在哪里停留时间最长。在企业和政府场景中,把这个目标与领导层已经在追踪的流动度量挂钩,因为一项没有延迟规范的强制多人评审控制,会变成冻结交付、并诱使人们完全绕开控制的瓶颈。

  5. 对于哪些类型的变更,你们信任自动化和 AI 辅助评审,在哪些地方必须由人来承担责任? 本章指出,要把 AI 评审的输出当作输入,而不是权威结论:它擅长表层问题和一致性检查,不擅长深层的设计判断和系统语境理解,每一次批准都要由一个人负责。没有明确的边界,大型团队会逐渐滑向过度信任,一条绿色的机器人评论读起来就像是通过了评审,真正的设计和安全风险却在虚假的信心中溜走。与之对立的拉力是,AI 评审确实能减轻负担,不知疲倦地捕获常见缺陷,所以完全禁止它就是在浪费杠杆效应。带上证据:自动化建议在哪些地方捕获了真正的问题,在哪些地方只是制造了噪音,以及哪些变更类型(安全敏感的、合规相关的、架构性的)你们绝不会让机器独自签核。对于企业和政府相关工作,要明确当一个 AI 助手参与其中时,谁对这次批准负责,因为审计时会有人问是谁评审了这项变更,「是工具评审的」不是监管机构能接受的答案。

  6. 在哪些地方,结对或群组编程应该取代异步评审,你们如何有意识地利用评审来降低巴士因子风险? 本章把结对编程和群组编程定位为根据情境选择的持续性评审,并把评审定位为传播知识的机制,使系统的任何一个部分都不会只被一个人理解。如果任其自然,知识就会集中:同一位专家评审某个子系统的每一次变更,评审就变成了橡皮图章,因为没有其他人能够质疑他们,而巴士因子风险恰恰在系统最关键的地方不断增长。与之对立的考量是成本,因为群组编程会耗费整个团队的时间,结对编程会占用两名工程师,所以你不能在所有地方强制推行它。带上证据:哪些模块只有一位可信的评审者,哪里的入职引导停滞不前,哪个棘手的领域会从一场实时会话中获益,而不是靠评论串。在大型组织或公共机构中,要把有意识地传播知识当作一种风险管理手段,因为一个关键部分依赖单一个人的长生命周期系统,是一项运营和连续性方面的负债,而不仅仅是人员配置上的不便。

行业视角

初创企业。 三四名工程师的规模下,让评审保持轻量:一位同事对小型拉取请求的批准,CI 中的机械性检查,不设会拖慢合并的强制第二评审人。真正的目标与其说是合规,不如说是确保不止一个人理解系统的每一部分,所以在风险较高的部分进行结对,并把它当作入职引导来对待。不要构建你们很快就会用不上的繁重代码所有权路由;一种小型、描述清晰的变更的共享规范,几乎不花什么成本就能带来大部分好处。

小型企业。 你们不太可能拥有一位评审工具专家,所以要依靠你们的托管平台(例如一项托管的 Git 服务)开箱即用提供的功能,而不是自建定制的自动化系统。购买代码检查、测试和安全扫描的集成方案,而不是自己维护它们,这样你们为数不多的工程师就能把稀缺的评审时间花在设计和正确性上。保持一条简单的规则:每一项变更都要有另一双眼睛过目,抵制住添加没有人手维护的流程的冲动。

企业。 挑战在于跨众多团队的一致性:共享标准、把敏感变更路由给正确审批人的代码所有权规则,以及被记录为审计证据的基于角色的审批。在整个组织范围内自动化机械性检查,让人工评审集中在设计上,并把评审延迟作为一项流动度量来追踪,这样强制多人评审控制就不会悄悄变成瓶颈。用一份有文档记录的政策,让评审深度与变更风险相匹配,这样琐碎的变更能保持快速,而高风险的变更则能获得职责分离和更深入的审查。

政府部门。 变更控制往往是强制性的:每一次生产环境变更都要由作者之外的人评审和批准,记录被保留作为审计证据,以满足职责分离要求。要偏好一条透明、可追溯的记录,记录谁编写了代码、谁批准了它、哪些检查通过了,并投资于自动化和小型、频繁的变更,这样这项控制就不会冻结交付。在采购评审工具时,要求可导出的审计日志,避免供应商锁定,因为证据必须比任何单一供应商都存在得更久,并能经受住公众的审视。

示例

初创企业。 一家四人工程师团队的初创公司让每一个拉取请求都保持小巧,并要求合并前获得一位同事的批准,这与其说是为了合规,不如说是为了确保不会只有一个人理解系统的某个部分。CI 运行格式化工具和测试,因此人工只需把他们有限的评审时间花在设计和正确性上,而不是空格排版。当团队遇到支付流程中一处棘手的部分时,其中两人选择结对处理它,而不是交换异步评论,这同时也充当了最新入职员工的入职引导。

企业。 一家大型软件公司要求每一项变更至少获得一次批准评审,对由代码所有权规则识别出的安全敏感模块的变更,还要求第二次批准。CI 处理所有的风格和测试检查,因此评审者专注于设计和正确性。团队追踪首次评审耗时,把中位数的上升当作重新平衡工作负载的信号。新工程师通过结对编程完成入职引导,这缩短了他们独立贡献的路径。

政府部门。 一家在严格变更控制要求下运作的国家机构,要求每一次生产环境变更都由作者之外的人评审和批准,批准记录被保留用于审计。为防止这项控制变成瓶颈,该机构投资于自动化检查和小型、频繁的变更,并设定了当日响应的评审规范。评审记录()涵盖谁编写了代码、谁批准了它、哪些检查通过了()成为每次发布合规证据的一部分,在不冻结交付的情况下满足了职责分离要求。

商业论证:动机、投资回报率与总拥有成本

代码评审以三种「货币」回报你:在生产环境之前捕获的缺陷、在团队中传播的知识,以及随时间自动维持的标准。在评审中捕获一个缺陷,远比在生产环境中捕获它便宜得多,而知识共享带来的好处,则降低了关键人员风险()一旦某人离职,这种风险原本可能让组织付出高昂代价。评审同时也是让不断壮大的团队保持凝聚力的文化传递机制。

评审的成本是工程师的时间和一定的延迟,这两者在良好实践下都是可控的。而不进行评审、或评审得糟糕的代价,包括生产环境缺陷、知识孤岛、不一致的代码,以及在受监管场景中,审计失败和合规问题。过度沉重的评审同样也有其真实成本:漫长的队列、超大的批次、士气低落的工程师。要向领导层论证,把评审实践与变更失败率、交付周期和入职速度联系起来,并把评审延迟作为一项明确的流动度量来追踪。

反模式与陷阱

  • 橡皮图章: 没有真正审查就予以批准,提供虚假的安心感,只满足控制要求的表面文字。
  • 超大 PR: 数千行代码,只能被粗略浏览,注定得到浅层评审。
  • 只挑刺式评审: 专注于琐碎细节,却错过设计和正确性问题,往往是因为机械性检查没有被自动化。
  • 把评审当作把关: 利用评审来彰显权威或阻挠他人,毒化协作氛围。
  • 缓慢的队列: 评审搁置数天,拖慢交付,并鼓励批量提交。
  • 过度信任 AI 评审: 把自动化建议当作权威结论,在风险变更上放弃人的判断。
  • 以偏好为由阻塞: 把个人风格偏好当作必需的变更提出来,却不将其与真正的缺陷区分开。

成熟度模型

  • 第一级,启动: 评审是临时应对、被动反应的。评审常被跳过或执行不一致,评论以机械性问题为主,反馈规范尚未确立,任何批准记录都是偶然产生的,而非刻意设计的结果。
  • 第二级,发展: 基本的评审实践已经存在,但因团队而异。评审在一些地方是强制性的,在另一些地方则缓慢或可选,自动化只覆盖了部分环节,拉取请求的规模和质量参差不齐,没有共享的预期标准。
  • 第三级,标准化: 标准已被文档化,并在全组织范围内强制执行。包括小型聚焦的 PR、CI 中自动化的格式化、代码检查、测试和安全扫描、清晰的检查清单、明确的阻塞性与建议性的区分约定,以及把敏感变更路由给正确审批人的代码所有权规则。
  • 第四级,管理: 评审对照基线被度量和控制。追踪首次评审耗时、合并耗时、评审深度与变更风险的匹配度、缺陷逃逸率和变更失败率;持续存在的延迟被当作流程问题处理;数据驱动决定在哪里重新平衡评审者的工作负载,以及哪些控制在拖慢交付却没有带来额外保障。
  • 第五级,协同: 评审在整个组织范围内被持续改进和整合。深度随变更风险自适应调整,结对、群组编程和 AI 辅助被有意识地使用,并始终有一个人负责,知识传播和巴士因子风险被有意识地管理,评审能够可衡量地提升质量、交付流动性和入职引导效果。

讨论议题

  • 对你们团队而言,合适的评审延迟目标是什么,是什么阻止了你们达成它?
  • 你们如何在不增加官僚负担的情况下,让评审深度与变更风险相匹配?
  • 在你们的场景中,结对或群组编程在哪些地方胜过异步评审?
  • AI 辅助评审应该被信任到什么程度,适用于哪些类型的变更?
  • 随着团队规模扩大、构成日益多元,你们如何保持评审反馈的建设性?
  • 你们如何在不制造瓶颈的情况下满足合规审批要求?

关键要点

  • 让拉取请求保持小巧且描述清晰;可评审性由作者负责。
  • 把机械性工作自动化,让人来评审设计、正确性和测试。
  • 把评审延迟作为一项团队范围内的流动成本来追踪和管理。
  • 让评审深度与变更风险相匹配,并区分阻塞性问题和偏好。
  • 把结对编程、群组编程和 AI 辅助当作因情境而定的补充手段,并始终让一个人负责。

参考文献与延伸阅读

  • Karl Wiegers, Peer Reviews in Software: A Practical Guide
  • Google, Engineering Practices: How to Do a Code Review (as a reference exemplar)
  • Nicole Forsgren, Jez Humble, Gene Kim, Accelerate: The Science of Lean Software and DevOps
  • Kent Beck, Extreme Programming Explained (on pair programming)
  • Woody Zuill, writings on mob programming
  • Michael Lopp, Managing Humans (on engineering collaboration)