Google Engineering Practices 文档仓库导读深入解析 Google 代码评审指南Code Review Guidelines【免费下载链接】eng-practicesGoogles Engineering Practices documentation项目地址: https://gitcode.com/gh_mirrors/eng/eng-practices本仓库eng-practices是 Google 官方工程实践文档的开源镜像以 README.md 为总入口收录了 Google 长期沉淀的、跨语言跨项目的通用工程最佳实践。其核心主体是两份互为补充的代码评审指南——面向评审人的The Code Reviewers Guide与面向变更作者的The CL Authors Guide外加一份紧急情况处理说明。读完本文你将掌握 Google 代码评审的完整流程、评审人应遵循的标准与逐行检查清单、变更作者撰写 CLchangelist描述与拆分小 CL 的实战方法以及 CL、LGTM 等 Google 内部术语的确切含义可直接迁移到任何团队的日常评审实践中。一、仓库概览Google 工程实践文档讲什么根据 README.md 的说明Google 拥有大量覆盖所有语言和所有项目的通用工程实践这些文档代表了 Google 在长期开发中积累的集体经验。Google 认为开源项目或其他组织同样能从这些知识中获益因此尽可能将其公开。当前仓库包含的核心文档是 Google 的代码评审指南它实际上由两套相互独立的文档组成评审人指南The Code Reviewers Guide为代码评审人提供详细指导变更作者指南The CL Authors Guide为正在走评审流程的开发人员提供详细指导。两套指南的定位互补评审人指南教怎么看作者指南教怎么被看。仓库同时收录了 emergencies.md专门界定紧急 CL的标准与处理方式。二、术语表CL 与 LGTM 的含义文档中使用了部分 Google 内部术语README.md 为外部读者做了专门澄清CL即 changelist指一个自包含的变更该变更已被提交到版本控制或正在接受代码评审。其他组织通常称之为 change、patch 或 pull-request拉取请求。LGTM即 Looks Good to Me是代码评审人批准一个 CL 时所说的话相当于我看没问题可以合入。这两个术语贯穿整个文档体系。理解它们之后再读评审人指南与作者指南就不会有理解障碍。三、代码评审总览流程定义与评审维度review/index.md 给出了代码评审的权威定义代码评审是一个由代码作者以外的人来检查代码的过程。在 Google代码评审被用于维持代码和产品的质量这份文档是整个评审流程与策略的权威描述。3.1 评审人应该关注什么评审应该覆盖以下八个维度详见 review/index.md 与 评审人指南的展开讲解设计Design代码设计是否良好、是否适配当前系统功能Functionality代码行为是否符合作者预期、对用户是否友好复杂度Complexity代码能否再简化未来其他开发者能否快速理解并使用测试Tests是否包含正确且设计良好的自动化测试命名Naming变量、类、方法等是否命名清晰注释Comments注释是否清晰、有用风格Style是否遵循 Google 官方 Style Guide文档Documentation是否同步更新了相关文档。3.2 如何挑选最佳评审人评审人选择的原则是找到能力范围内能给出最全面、最正确评审意见的人并且此人能在合理时间内响应。通常这意味着代码的 owner拥有者——他们可能是也可能不是 OWNERS 文件里列出的人。有时候一个 CL 的不同部分需要请不同的人评审。如果找到了理想的评审人但对方暂时没空至少要把他CC抄送到你的变更上保证对方知情。3.3 面对面评审与结对编程如果开发者与一位具备评审资格的人**结对编程pair programming**完成了某段代码那么这段代码可以视为已被评审过。此外也可以进行面对面评审评审人提问变更作者只在被问到的时候发言。这两种形式是异步评审工具之外的有效补充。四、评审人指南六个主题的完整实践review/reviewer/index.md 说明这一部分的页面基于长期经验给出了做代码评审的最佳方式。所有页面合在一起是一篇完整文档拆成了六个独立小节。下面按顺序逐一展开。4.1 评审标准追求更好的代码而非完美的代码standard.md 是所有代码评审指南中最核心的高级原则the senior principle。代码评审的首要目的是确保整个代码库的健康度随时间持续改善所有工具和流程都为此服务。为此需要在两组诉求之间做权衡开发者必须能够取得进展——如果从不提交改进代码库永远不会变好如果评审人让任何变更都难以合入开发者就会失去改进的动力评审人有责任确保每个 CL 的质量不会让代码库健康度随时间下降——代码库往往正是通过一次次小的健康度下降而劣化的尤其在团队时间压力大、不得不走捷径的时候。由此得出评审标准的黄金法则一般来说只要一个 CL 确实能改善所开发系统的整体代码健康度评审人就应倾向于批准它即使这个 CL 并不完美。同时要明确边界不存在完美的代码只有更好的代码。评审人不应要求作者打磨 CL 的每一个细节才给批准而应平衡向前推进的需求与所提建议的重要性追求的是持续改进continuous improvement。一个整体上提升了可维护性、可读性、可理解性的 CL不应因为不够完美而被拖延数天或数周。评审人可以随时留下可以更好的意见但如果不重要就用Nit:前缀标注让作者知道这只是可选打磨点。需要特别强调的是本文档不允许合入确定会损害代码库健康度的 CL——唯一的例外是紧急情况。此外评审还具有**指导Mentoring**功能教会开发者关于语言、框架或软件设计原则的新知识本身也是改善代码库健康度的一部分。纯教育性、非强制性的意见同样用Nit:标注。评审原则Principles技术事实和数据优先于个人意见和偏好风格问题以 Style Guide 为绝对权威不在 Style Guide 内的纯风格点如空白符属于个人偏好应与已有代码保持一致无既有风格时接受作者的软件设计的方面几乎从来不是纯风格或个人偏好问题它们基于底层原则应依据原则权衡。如果作者能证明多个方案同样有效基于数据或扎实的工程原则评审人应接受作者的选择否则按标准软件设计原则决定若无其他规则适用评审人可以要求作者与当前代码库保持一致——前提是不损害代码库整体健康度。冲突解决任何评审冲突的第一步都是开发者和评审人依据本文档、作者指南和评审人指南尝试达成共识。难以达成时可安排面对面或视频会议讨论讨论结果务必以注释形式记录在 CL 上方便未来读者仍无法解决则升级escalate常见路径包括团队范围讨论、请技术负责人TL介入、请代码维护者裁决或请工程经理Eng Manager协助。关键原则是不要因为作者与评审人无法达成一致而让 CL 悬置不动。4.2 评审看什么逐行检查清单looking-for.md 对评审的各个维度做了深度展开评审人应始终结合评审标准来权衡每一条。设计评审中最重要的是 CL 的整体设计。各部分代码的交互是否合理这个变更属于当前代码库还是应该放进库library是否与系统其余部分集成良好现在是否是添加该功能的合适时机功能CL 是否做了开发者想做的事用户通常既包括终端用户受变更影响时也包括未来要使用这些代码的开发者。大多数情况下CL 到达评审时应当已被测试得足够好但评审人仍要主动思考边界情况、排查并发问题、站在用户角度检查并寻找仅靠读代码就能发现的 bug。评审人也可以自行验证 CL——当变更对用户有可见影响尤其是UI 变更时最值得验证若涉及并行编程可能引发死锁或竞态条件务必仔细推演这类问题很难仅靠运行代码发现这也是避免使用易产生竞态/死锁的并发模型的理由。复杂度CL 是否比应有的更复杂要在每一层检查——单行、函数、类。过于复杂通常意味着读者无法快速理解或开发者调用/修改时容易引入 bug。特别要警惕过度工程over-engineering代码被做得比需要的更通用或添加了当前系统不需要的功能。鼓励开发者解决现在确实需要解决的问题而不是推测未来可能需要的问题。测试按需要求单元、集成或端到端测试。测试通常应和生产代码放在同一个 CL紧急情况除外。要确认测试正确、合理、有用——测试不会测试自己必须由人来确保测试有效代码坏了时测试真的会失败吗代码变更后会产生误报吗每个测试的断言是否简单有用测试方法之间是否恰当分离记住测试也是需要维护的代码不要因为测试不在主二进制中就接受其复杂度。命名名字应足够长以完整传达含义又不过长到难以阅读。注释注释应清晰且用可理解的英文书写并且只在必要时存在。注释通常应解释代码为什么存在why而不是解释代码在做什么what——代码不清楚就该简化代码。正则表达式和复杂算法是例外它们常受益于解释在做什么的注释。同时留意 CL 之前就存在的注释是否有可以删除的 TODO、是否有劝阻本次变更的注释等。注意注释不同于类/模块/函数的文档documentation——后者应表达代码的目的、用法和行为。风格所有主要语言都有 Style Guide。想改进 Style Guide 之外的个人风格点用Nit:前缀标注且不要仅凭个人风格偏好阻止 CL 合入。作者不应把大规模风格改动与其他改动混在同一个 CL 里——这会让人难以看清变更内容、使合并和回滚更复杂。例如要重排整个文件格式应单独发一个纯格式化 CL再发下一个功能变更 CL。一致性若既有代码与 Style Guide 不一致按评审原则Style Guide 是绝对权威。当 Style Guide 只是建议而非要求时需要判断新代码跟随建议还是跟随周围代码——倾向跟随 Style Guide除非局部不一致会过于令人困惑。无论如何鼓励作者为清理既有代码提 bug 并加 TODO。文档如果 CL 改变了用户构建、测试、交互或发布代码的方式检查它是否同步更新了相关文档README、内部文档页、生成的参考文档。删除或废弃代码时考虑文档是否也应删除。文档缺失就要求补上。每一行Every Line一般情况下要检查分配给你的每一行代码。数据文件、生成代码、大型数据结构可以略扫但不要略过人工编写的类、函数或代码块并假设里面没问题。如果代码太难读导致评审变慢应告知开发者先澄清再评审——在 Google 招聘的都是优秀工程师如果你看不懂其他开发者很可能也看不懂。如果你理解了代码但对某部分如隐私、安全、并发、无障碍、国际化不够资格评审确保 CL 上有合格的人来评审。例外情况当你是多个评审人之一、只被要求评审部分文件或部分方面如高层设计、隐私、安全时在注释中说明你评审了哪些部分优先给出带评论的 LGTM。若要在确认其他评审人已覆盖其余部分后给出 LGTM也请在注释中明确说明以设定预期。上下文Context评审工具通常只展示变更周围的几行代码有时必须看整个文件才能确认变更是否合理例如新增的 4 行代码位于一个 50 行的方法中该方法其实需要拆分。同时把 CL 放在系统整体背景下思考它是在改善系统健康度还是让系统更复杂、测试更少不要接受损害系统代码健康度的 CL——系统大多是通过许多小变更累积而变得复杂的因此要防止新变更中哪怕微小的复杂度。好的地方Good Things如果看到 CL 中的亮点要告诉开发者——尤其是当他们很好地处理了你的某条意见时。评审往往只关注错误但也应该给予鼓励和欣赏。从指导角度看告诉开发者做对了什么有时比指出做错了什么更有价值。最后looking-for.md 给出了评审结论自检清单代码设计良好、功能对用户友好、UI 变更合理美观、并行编程安全、代码没有过度复杂、没有实现未来可能需要的东西、有合适的单元测试且测试设计良好、命名清晰、注释清晰有用且多解释 why、代码有恰当文档、符合 Style Guide。记住逐行检查、看上下文、确保在改善代码健康度、称赞开发者做得好的地方。4.3 浏览一个待评审的 CL三步走navigate.md 回答了面对跨多个文件的评审最高效的浏览方式是什么给出三步流程从整体上审视变更先看 CL 描述和 CL 大致做了什么。这个变更本身合理吗如果这个变更根本不该发生请立即回复并解释原因同时建议开发者应该改做什么。例如看起来你在这上面下了不少功夫谢谢不过我们正在朝移除你修改的 FooWidget 系统的方向走暂时不想对它做任何新修改。不如你去重构新的 BarWidget 类——注意这个例子不仅拒绝了 CL 并给出替代建议而且彬彬有礼。如果你频繁收到你不想做的变更的 CL应考虑重新设计团队开发流程或外部贡献者流程在 CL 写出之前多做沟通——在别人付出大量即将被丢弃或重写的劳动之前说不。先看 CL 的主要部分找到主文件——通常有一个文件包含最多逻辑变更。先看主要部分能为其余小部分提供上下文加速评审。如果 CL 太大以至于分不清主次问开发者该先看什么或请他们把 CL 拆成多个 CL。如果发现主要设计问题要立即发出意见即使没时间评审其余部分——如果设计问题足够严重其余代码大概率会消失继续评审就是浪费时间。立即发出设计意见有两大理由开发者常常在发出 CL 后立即基于它开始新工作及早指出能避免他们在有问题的设计上多做无用功且大改比小改耗时更长为了赶上截止日期开发者需要尽快开始返工。按合适顺序浏览其余部分确认整体没有重大设计问题后按逻辑顺序浏览文件同时确保不遗漏任何文件。通常看完主要文件后按评审工具呈现的顺序逐个过文件最简单有时先读测试再读主代码也很有帮助因为测试能让你先了解变更应该做什么。4.4 评审速度团队速度优先于个人速度speed.md 开宗明义Google 优化的是一个开发团队共同生产产品的速度而不是单个开发者写代码的速度。个人开发速度也重要但没有整个团队的产出速度重要。为什么评审要快评审慢会引发三个连锁反应团队整体产出速度下降不快速响应的人确实能做其他工作但团队的新功能、bug 修复会随着每个 CL 等待评审和复审而推迟数天、数周甚至数月开发者开始抵制评审流程如果评审人每隔几天才响应一次、每次都要求大改开发者就会抱怨评审人太严格。如果评审人要求的是同样的大改但每次都快速响应抱怨通常就会消失——大多数对评审流程的抱怨其实是通过让流程变快而解决的代码健康度受损评审慢时允许开发者提交质量不佳 CL 的压力会增加慢评审也会抑制代码清理、重构和对既有 CL 的进一步改进。评审要多快不在专注任务中时应在 CL 到来后不久就进行评审一个业务工作日是响应评审请求的最长时限即次日上午第一件事遵循以上原则一个典型的 CL 在一天内应能完成多轮评审如需要。速度与中断的权衡唯一个人速度胜过团队速度的情形是你正处于写代码之类的专注任务中时不要中断自己去做评审。研究表明被打断后开发者需要很长时间才能重新进入流畅的开发状态中断自己的代价比让另一个开发者多等一会儿评审更大。应在工作的断点响应评审请求——当前编码任务完成时、午饭后、开完会回来、休息回来时。快响应而非快全流程评审速度关注的是响应时间而不是一个 CL 走完全程被提交的时间。整个过程当然也应当快但单个响应的快速比整个流程的快速更重要。即使整个评审流程偶尔耗时较长评审人的快速响应也能显著缓解开发者对慢评审的挫败感。如果太忙无法完整评审可以快速回复告知何时处理、建议更快的其他评审人或给出一些初步的整体意见注意这些也应在合理断点发送不要中断编码。同时评审人必须花足够时间确保自己的 LGTM 意味着这份代码符合我们的标准。跨时区评审面对时区差异尽量在作者下班前回复让他们还有时间响应如果对方已下班则确保在他们次日开工前完成评审。带评论的 LGTMLGTM With Comments为加速评审以下两种情况评审人应给出 LGTM/批准尽管 CL 上还留有未解决评论评审人确信开发者会妥善处理所有剩余评论剩余变更都是次要的、不一定要做的。评审人应说明自己属于哪种情况若不够明确。这种方式在开发者和评审人跨时区、开发者可能为了一个 LGTM, Approval 白白等一整天时尤其值得考虑。大 CL 的处理收到大到不知何时才能审完的 CL典型回应是请开发者把它拆成一串相互依赖的小 CL而不是一次性审一个巨型 CL。这通常可行且对评审人很有帮助即使需要开发者额外做些工作。如果 CL 实在拆不开且没时间快速审完至少就整体设计写些评论发回给开发者改进。评审人的目标之一是在不牺牲代码健康度的前提下始终尽快为开发者解阻或让其能继续行动。随时间推移评审会越来越快遵循这些准则并严格评审整个评审流程会随时间越来越快开发者学会了健康代码的要求一开始就交出高质量的 CL评审人学会快速响应、不引入无谓延迟。但不要为了想象出来的速度提升而牺牲评审标准或质量——从长远看那并不会让任何事更快。紧急情况也存在必须让 CL整个评审流程飞速通过的紧急情况此时质量准则可以放宽——但请务必查阅 什么是紧急情况了解哪些情形真正算紧急、哪些不算。4.5 如何写评审评论礼貌、解释理由、标注严重程度comments.md 的要点总结为四条友善解释你的理由在给出明确指示与只指出问题让开发者决定之间取得平衡鼓励开发者简化代码或添加注释而不是向你解释复杂度。礼貌Courtesy在清晰、有帮助的同时保持礼貌和尊重。一个关键技巧是评论永远针对代码而不是针对开发者。例如不好为什么你在这里用线程显然并发没有任何好处 好这里的并发模型给系统增加了复杂度而我没看到任何实际性能收益。因为没有性能收益这段代码最好保持单线程而不是使用多线程。解释为什么Explain Why好的例子会让开发者理解你评论的原因——你的意图、你遵循的最佳实践、或你的建议如何改善代码健康度。给予指导Giving Guidance修复 CL 的责任在开发者不在评审人你不必为开发者做详细设计或写代码。但评审人也不应不帮忙要在指出问题与给出直接指导之间取得平衡。指出问题让开发者自己决策通常能帮助开发者学习、让评审更顺畅还可能产生更好的方案因为开发者更贴近代码。但有时直接指示、建议甚至贴出代码更有帮助——评审的首要目标是拿到最好的 CL次要目标是提升开发者技能让他们未来需要越来越少评审。同时记住人们会从做对了什么的正向强化中学习开发者清理了混乱算法、补充了优秀的测试覆盖、或你从 CL 中学到了新东西都值得评论出来并同样附上 why。标注评论严重程度Label comment severity考虑用标签区分必须改与建议/可选。示例Nit小问题。技术上应该做但影响不大。 Optional或 Consider我觉得这可能是个好主意但不是硬性要求。 FYI我不要求你在本 CL 中处理但你可能觉得值得为未来想一想。这让评审意图明确帮助作者区分各条评论的优先级也避免误解——没有标签时作者可能把所有评论都当成必须处理的。接受解释Accepting Explanations如果你请开发者解释一段看不懂的代码通常应该促使他们把代码重写得更清晰偶尔在代码中加注释也是合适的回应但不应只是解释过度复杂的代码。只写在评审工具里的解释对未来代码读者没有帮助只在少数情况下可接受例如你在评审一个不太熟悉的领域、开发者解释的是正常读者本应知道的内容。4.6 处理评审中的反驳pushback.md 面向开发者不同意你的建议或抱怨你太严格的情形。谁是对的开发者反驳时先花点时间想想他们是不是对的。他们往往比你更贴近代码可能对某些方面有更好的洞察。他们的论证是否合理从代码健康角度是否说得通如果成立就承认他们是对的并放下此事。但开发者不总是对的——此时评审人应进一步解释为何自己的建议正确好的解释既体现你理解了开发者的回复又提供关于为何提出该变更的额外信息。特别是当评审人相信自己的建议会改善代码健康度时如果认为由此带来的质量提升值得额外工作就应继续坚持——代码健康度的改善往往靠小步积累。有时要解释好几轮才能让对方真正理解过程中保持礼貌并让开发者知道你听懂了他们的意思只是不同意。让开发者不高兴评审人有时担心坚持改进会让开发者不快。开发者确实偶尔会不快但通常是短暂的之后往往会感谢你帮他们提升了代码质量。只要评论写得礼貌开发者通常根本不会生气——不快更多源于评论的写法而非评审人对质量的坚持。以后再清理的陷阱开发者可以理解地想快点完事不想为这个 CL 再走一轮评审于是说以后在另一个 CL 里清理要求现在就给 LGTM。经验表明作者写完原 CL 后时间过得越久清理越不可能发生——除非开发者在本 CL 之后立即做清理否则几乎永远不会做。这不是因为开发者不负责任而是因为他们工作太多清理在忙碌中被遗忘。因此通常最好坚持让开发者在代码进入代码库、完成之前现在就清理。让事情以后再清理是代码库劣化的常见途径。若 CL 引入了新复杂度除非是紧急情况必须在提交前清理若 CL 暴露了周围的问题而当下无法解决开发者应为其提 bug 并指派给自己以免丢失可选地在代码中加一条引用该 bug 的 TODO。关于太严格的普遍抱怨如果之前评审宽松、现在转严有些开发者会强烈抱怨。提高评审速度通常能让抱怨消退有时需要数月但开发者最终会看到严格评审带来的优秀代码的价值——最激烈的抗议者甚至可能在真正看到价值后变成最坚定的支持者。冲突解决遵循以上做法仍无法解决冲突时回到评审标准中的原则和准则。五、变更作者指南三个主题的实战方法review/developer/index.md 面向正在经历评审的开发者目的是让评审走得更快、结果质量更高。这些准则适用于每一位 Google 开发者共三个主题。5.1 写好 CL 描述版本历史中的公共记录cl-descriptions.md 强调CL 描述是关于改了什么和为什么改的公共记录它将永久留在版本控制历史中未来多年可能被除评审人外的成百上千人阅读。未来的开发者会根据描述搜索你的 CL——可能只凭对相关性的模糊记忆。如果所有重要信息都埋在代码里而不是描述里他们找到你的 CL 会困难得多。第一行简洁总结做了什么完整的祈使句命令式句子后面跟一个空行。第一行是 CL 描述中出现在版本控制历史摘要里的部分应足够信息丰富让未来搜索者不必读 CL 或全文描述就能明白它做了什么、与其他 CL 有何不同——即第一行要独立成立让读者能快速浏览代码历史。保持简短、聚焦、切中要点清晰和实用是第一位的。按传统第一行是写成命令式的完整句子例如说删除FizzBuzz RPC 并替换为新系统而不是删除……Deleting。其余部分不必都用祈使句。正文要有信息量第一行做简短聚焦的总结正文则补充细节和读者全面理解该变更所需的附加信息可包括所解决问题的简要描述、为什么这是最佳方案、方案的不足之处、相关背景bug 编号、基准测试结果、设计文档链接。注意外部资源链接可能因访问限制或保留策略而无法被未来读者看到尽可能在描述中放入足够上下文。即使小 CL 也值得注意细节——把 CL 放进上下文里。糟糕的 CL 描述Fix bug 是不合格的描述——什么 bug你怎么修的类似的坏例子还有Fix build.、Add patch.、Moving code from A to B.、Phase 1.、Add convenience functions.、kill weird URLs.。这些有些是真实出现过的描述虽然短但提供不了足够有用的信息。好的 CL 描述三类实例功能变更示例RPC: Remove size limit on RPC server message freelist.Servers like FizzBuzz have very large messages and would benefit from reuse. Make the freelist larger, and add a goroutine that frees the freelist entries slowly over time, so that idle servers eventually release all freelist entries.前几个词说明 CL 实际做了什么其余部分讲要解决的问题、为什么是好方案以及具体实现细节。重构示例Construct a Task with a TimeKeeper to use its TimeStr and Now methods.Add a Now method to Task, so the borglet() getter method can be removed (which was only used by OOMCandidate to call borglets Now method). This replaces the methods on Borglet that delegate to a TimeKeeper.Allowing Tasks to supply Now is a step toward eliminating the dependency on Borglet. Eventually, collaborators that depend on getting Now from the Task should be changed to use a TimeKeeper directly, but this has been an accommodation to refactoring in small steps.Continuing the long-range goal of refactoring the Borglet Hierarchy.第一行说明 CL 做了什么、与过去有何不同其余部分讲具体实现、CL 上下文、方案并非完美以及未来方向还解释了为什么做这个变更。需要上下文的小 CL示例Create a Python3 build rule for status.py.This allows consumers who are already using this as in Python3 to depend on a rule that is next to the original status build rule instead of somewhere in their own tree. It encourages new consumers to use Python3 if they can, instead of Python2, and significantly simplifies some automated build file refactoring tools being worked on currently.第一句说明实际做了什么其余部分解释为什么并给评审人大量上下文。使用标签Tags标签是手动输入的、用于给 CL 分类的标记可能由工具支持也可能只是团队约定。例如[tag]、[a longer tag]、#tag、tag:。使用标签是可选的。添加标签时要考虑应该放进正文还是第一行——限制第一行中的标签数量以免遮蔽内容。示例// 短标签可以放在第一行 [banana] Peel the banana before eating. // 标签可以内联在内容中 Peel the #banana before eating. // 标签是可选的 Peel the banana before eating. // 保持简短时多个标签也可接受 #banana #apple: Assemble a fruit basket. // 标签可以放在 CL 描述的任何位置 Assemble a fruit basket. #banana #apple反面示例过多或过长的标签会压垮第一行——应考虑把标签移到正文并/或缩短如[banana peeler factory factory][apple picking service] Assemble a fruit basket.就是坏的示范。工具生成的 CL 描述与提交前复查有些 CL 由工具生成其描述也应遵循上述建议第一行简短、聚焦、独立成立正文包含有助于评审人和未来搜索者理解的信息。此外CL 在评审中可能发生重大变化提交前复查一遍描述确保它仍准确反映 CL 实际做了什么。5.2 小 CL一个自包含的变更small-cls.md 是小而简单的 CL 的完整指南。为什么写小 CL小 CL 有八大优势评审更快评审人更容易多次挤出 5 分钟来审小 CL而不是专门腾出 30 分钟审一个大 CL评审更彻底大变更中评审人和作者容易被大量来回的详细评论搞得疲惫甚至漏掉重点更少引入 bug改动少你和评审人都更容易有效推理 CL 的影响、发现是否引入 bug被拒时浪费更少大 CL 被说整体方向错了时大量工作白白浪费更容易合并大 CL 耗时长、合并冲突多、需要频繁合并更容易设计好打磨小变更的设计和代码健康度比精修大变更的每个细节容易得多更少被评审阻塞发送自包含的变更片段你可以在等待当前 CL 评审的同时继续编码回滚更简单大 CL 更可能触碰初始提交与回滚之间被更新的文件使回滚复杂化中间 CL 可能也需要一起回滚。特别提醒评审人有权仅以变更太大为由直接拒绝你的 CL。通常他们会感谢你的贡献但要求你把它拆成一串小变更。已经写好的变更再拆分很费工或要花大量时间争论为什么评审人该接受大 CL——不如一开始就写小 CL。多大算小一般来说CL 的合适大小是一个自包含的变更意味着CL 做最小变更只解决一件事——通常只是一个功能的一部分而非整个功能一次完成宁小勿大与评审人沟通可接受的大小CL 应包含相关测试代码评审人理解 CL 所需的全部信息未来开发除外都在 CL、CL 描述、既有代码库或他们评审过的 CL 中CL 合入后系统对用户和开发者仍能正常工作CL 不能小到含义难以理解——例如新增 API 时应在同一 CL 中给出 API 的使用示例帮助评审人理解用法也防止合入无人使用的 API。没有关于多大算太大的硬性规则100 行通常是合理大小1000 行通常太大但最终取决于评审人的判断。变更横跨的文件数也影响大小200 行集中在一个文件可能没问题摊在 50 个文件里通常就太大了。请记住你从动笔起就与代码亲密接触评审人往往没有任何上下文——你觉得合适的大小对评审人可能已经难以招架。拿不准时写比你认为需要的更小的 CL。评审人很少抱怨 CL 太小。什么时候大 CL 可以接受少数情况下大变更没那么糟删除整个文件通常可以只算一行变更评审人不需要花太多时间有时大 CL 由你完全信任的自动重构工具生成评审人的工作只是验证并确认确实想要这个变更——这种 CL 可以更大但上述注意事项如合并、测试仍然适用。高效地写小 CL写完一个小 CL 就干等批准再写下一个会浪费大量时间。要找到不被评审阻塞的工作方式同时进行多个项目、找能立即响应的评审人、做面对面评审、结对编程或以允许你立即继续工作的方式拆分 CL。拆分 CL 的策略开始做会产生多个相互依赖 CL 的工作前先在高层面规划如何拆分和组织这些 CL。这不仅让作者更好管理也让评审人更轻松从而让评审更高效。具体策略有堆叠多个变更Stacking写一个小 CL 发出去评审然后立即开始写基于第一个 CL 的下一个 CL。大多数版本控制系统都支持某种方式做到这点按文件拆分Splitting by Files按需要不同评审人、但本身自包含的文件分组来拆。例如一个 CL 修改 protocol buffer另一个 CL 修改使用该 proto 的代码——必须先提交 proto CL 再提交代码 CL但两者可以同时评审这样拆也便于回滚配置/实验文件有时比代码变更更快推到生产。拆的时候不妨告知两组评审人对方 CL 的存在让他们有上下文水平拆分Splitting Horizontally创建共享代码或桩stub来隔离技术栈各层之间的变更。以计算器应用client、API、service、data model 四层为例共享的 proto 签名可以把 service 层和 data model 层互相抽象开API 桩可以把 client 代码与 service 代码的实现拆分、让它们独立推进。类似思路也可应用于更细粒度的函数或类级抽象垂直拆分Splitting Vertically与分层的水平方式正交把代码拆成更小的、全栈式的垂直功能块每个功能块是独立的并行实现轨道一部分轨道在等待评审反馈时其他轨道可以继续推进。回到计算器例子要支持乘法和除法可以分别作为独立的垂直子功能实现即使它们有重叠如共享按钮样式、共享校验逻辑水平垂直网格Splitting Horizontally Vertically把两种方式结合画成如下实现计划每个单元格是一个独立 CL从最底部的 model 层开始往上做到 client 层层Layer功能乘法功能除法Client添加按钮添加按钮API添加端点添加端点Service实现变换逻辑与乘法共享变换逻辑Model添加 proto 定义添加 proto 定义单独拆出重构重构最好与功能变更或 bug 修复分开成独立 CL。例如移动并重命名一个类应与修复该类中的 bug分开评审人才能更容易理解每个 CL 引入的变更。但像修正局部变量名这样的小清理可以并入功能变更或 bug 修复 CL——由作者和评审人判断重构大到什么程度会拖累当前 CL 的评审。把相关测试代码放进同一个 CLCL 应包含相关测试代码——此处的小指概念上聚焦不是简单的行数函数。Google 的所有变更都期望有测试添加或修改逻辑的 CL 应伴随针对新行为的全新或更新测试纯重构 CL不打算改变行为也应被测试覆盖——理想情况下这些测试已存在不存在就要补上。独立的测试修改可以像重构指南那样先放进单独的 CL包括用新测试验证既有的已提交代码确保重要逻辑被测试覆盖增加后续重构的信心——例如想重构没有测试覆盖的代码时先提交测试 CL 再提交重构 CL可以验证重构前后行为不变、重构测试代码本身如引入辅助函数、引入更大的测试框架代码如集成测试。不要搞坏构建如果有多个相互依赖的 CL必须找到办法保证每个 CL 提交后整个系统仍能工作——否则两个 CL 提交之间的几分钟内或后续提交意外出错时的更长时间内你会让所有同事的构建挂掉。实在拆不小怎么办有时看起来 CL 不得不很大——这其实极少成立。坚持练习小 CL 的作者几乎总能找到把功能分解成一串小变更的方法。写大 CL 之前先考虑是否可以用一个纯重构 CL 开路、为更干净的实现铺路也可以问问队友有没有用多个小 CL 实现该功能的想法。如果所有方案都失败应该极其罕见则事先征得评审人同意再提交大 CL让他们有心理准备——这种情况下要做好长时间评审的准备警惕引入 bug加倍认真写测试。5.3 如何处理评审意见handling-comments.md 面向CL 发出去后收到一堆评审意见的场景。别往心里去评审的目的是维护代码库和产品的质量。评审人的批评是帮你、帮代码库、帮团队而不是对你个人的攻击。偶尔评审人会带着挫败情绪写评论——这对评审人不是好做法但你要有准备问问自己评审人想传达的建设性内容是什么然后按这个来行动。永远不要在愤怒中回复评审意见——那是对职业礼仪的严重违背且会永远留在评审工具里。太生气或太恼火无法友善回复时离开电脑一会儿或先做别的事等冷静下来再礼貌回复。如果评审人的反馈方式不建设性、不礼貌当面或视频通话或私人邮件礼貌说明你不喜欢什么、希望对方怎么做如果私下沟通后仍无效果可酌情向你的经理升级。先修代码评审人说看不懂你的某段代码时第一反应应该是澄清代码本身如果代码无法澄清就加一条解释代码为何存在的注释只有当注释看起来多余时才在评审工具里用文字解释。评审人看不懂的代码未来读者很可能也看不懂——写在评审工具里的回复帮不了未来读者澄清代码或加注释能帮到他们。协作式思考写一个 CL 可能花很多功夫发出后以为完事了却收到要求改动的评论确实令人沮丧。这时退一步想评审人是否在提供对代码库有价值的反馈第一问永远是我理解评审人在要求什么吗——不理解就问。理解但不同意时要协作式思考而不是对抗或防御式不好不我不会那么做。 好我选择 X 是因为[这些利弊]和[这些权衡]。我的理解是用 Y 会更糟因为[这些原因]。你是在说 Y 更好地服务了原有的权衡还是我们应该重新权衡这些权衡又或者是别的意思记住礼貌和尊重永远是第一优先级。不同意评审人时想办法协作请求澄清、讨论利弊、解释为什么你的做法对代码库/用户/团队更好。有时你知道一些评审人不知道的用户、代码库或 CL 的信息——该修代码就修同时给评审人更多上下文展开讨论通常基于技术事实就能达成共识。解决冲突第一步永远是尝试与评审人达成共识无法达成时参见评审标准中的处理原则。六、紧急情况什么算紧急什么不算emergencies.md 定义了必须让 CL整个评审流程以最快速度通过的紧急 CL。一个紧急 CL 是小变更且满足下列条件之一让重大发布得以继续而无需回滚、修复严重影响生产用户的 bug、处理紧迫的法律问题、堵上重大安全漏洞等。在紧急情况下团队真正关心的是整个评审流程的速度而不只是响应速度——只有在此时评审人才应该比其他一切更关心评审速度和代码正确性它是否真的解决了紧急问题。紧急评审出现时应优先于所有其他评审。紧急情况解决后要回头重新仔细评审这些紧急 CL参照评审看什么。明确不算紧急的情况包括想这周发布而不是下周除非有真正的硬性截止日期如合作伙伴协议开发者花了很长时间做某功能、非常想合入 CL评审人都在别的时区当地是夜晚或在外出差周五下班前想赶在开发者离开前合入 CL经理因为软截止日期要求今天必须审完合入回滚导致测试失败或构建损坏的 CL等等。什么是硬性截止日期是错过就会发生灾难性后果的截止日期例如按某日期提交 CL 是合同义务产品不在某日期前发布就会在市场彻底失败某些硬件厂商每年只发布一次新硬件错过向其提交代码的截止日期可能造成灾难。推迟一周发布不算灾难错过重要会议可能是灾难但常常不是。大多数截止日期是软截止日期——只是希望功能在某时间完成它们重要但不应为此牺牲代码健康度。如果发布周期很长数周很容易想牺牲评审质量把功能赶进下一个周期——但这种模式反复出现正是项目积累沉重技术债的常见途径。如果开发者经常在周期末尾提交必须合入的 CL 却只得到浅层评审团队就应该调整流程让大功能变更尽早进入周期、留足评审时间。七、许可协议与使用方式仓库中的文档采用 CC-By 3.0 许可鼓励共享这些文档详见 Creative Commons BY 3.0 条款。这意味着你可以在注明出处的前提下将本文档体系用于自己的团队、组织或开源项目例如直接采纳其中的评审标准、评论模板Nit:/Optional/FYI标签、CL 描述规范与小 CL 拆分策略让团队代码评审有据可依。仓库采用 Jekyll 主题见 _config.yml文档以 Markdown 编写结构清晰、便于直接阅读或二次分发。八、快速查阅路线图想了解整体结构README.md → review/index.md我是评审人评审人指南标准 → 看什么 → 浏览 CL → 速度 → 写评论 → 处理反驳我是变更作者作者指南CL 描述 → 小 CL → 处理评审意见遇到特殊情况紧急情况定义【免费下载链接】eng-practicesGoogles Engineering Practices documentation项目地址: https://gitcode.com/gh_mirrors/eng/eng-practices创作声明：本文部分内容由AI辅助生成（AIGC），仅供参考