文档教程【免费下载链接】eng-practicesGoogles Engineering Practices documentation项目地址https://gitcode.com/gh_mirrors/en/eng-practices点击查看免费下载本篇指南源自 Google 工程实践文档eng-practices 仓库中评审者指南的核心章节《What to look for in a code review》它系统回答了评审一个变更CL时应当逐项核查哪些维度这一问题。文章将结合仓库内 评审标准、评审速度、评审导航、评审评论写法 等姊妹文档为你梳理出一套可落地的评审检查清单从设计、功能、复杂度到测试、命名、注释、风格、一致性、文档再到逐行评审上下文视角肯定优点等实操要点。读完本文你将掌握一套完整的代码评审方法论知道在每个维度上应该问什么问题、用怎样的标准做判断以及如何在坚持质量的同时保持评审速度和团队协作的融洽。评审的总体前提先对齐评审标准原文档在开篇就强调了一条硬性前提在按下列任何一点展开评审之前永远要把《代码评审的标准》考虑在内。也就是说看什么之前先要明确用什么标准看。评审标准给出了这条链条的根基代码评审的首要目的是确保整个代码库的代码健康度随时间不断提升所有评审工具与流程都为此服务评审者需要平衡开发者能持续推进与代码健康度不下降这两端最终落到一条最高准则——只要一个 CL 整体上确实改善了系统的代码健康度即使它并不完美评审者也应倾向于批准它不存在完美代码只存在更好的代码评审者追求的是持续改进continuous improvement而不是为追求完美而把 CL 卡上数天或数周任何内容都允许以评论方式提出但如果它不那么重要就用 Nit: 前缀标明这只是可选打磨点唯一的例外是紧急情况没有任何东西授权你合入一个确定会恶化整体代码健康度的 CL。带着这个总纲下面进入本指南的主体——评审中需要逐项关注的内容。设计Design评审中最重要的一项原文档明确指出评审中最需要覆盖的就是 CL 的整体设计。当代码评审工具只展示几行改动时很容易陷入局部但设计问题恰恰需要在全局层面提问CL 中各个代码片段之间的交互是否有意义这个改动应该属于你的代码库还是应该放进某个库library它是否与系统其余部分集成良好现在是不是添加这项功能的好时机这四点分别对应内部逻辑自洽性、归属边界、系统集成度、时机判断。设计评审的结论往往决定整个 CL 的去留——正如 评审导航 所强调的如果发现重大设计问题应当立即发出设计评论即使你此刻没有时间评审其余部分——因为设计问题足够严重时CL 中的大量其他代码都会随之消失或重写继续评审剩余部分可能是浪费时间。功能Functionality面向用户的正确性功能维度要回答两个问题这个 CL 是否做了开发者想做的事开发者想做的事对代码的用户是否有益这里的用户通常有两类最终用户end-users——当改动直接影响他们时开发者developers——未来必须使用这段代码的人。评审者仍要主动想边界情况大部分情况下Google 期望开发者在提交评审之前已把 CL 测试到能正确工作。但作为评审者你依然要主动思考边界情况edge cases寻找并发问题concurrency problems尝试像用户一样思考通过读代码找出肉眼可见的 bug。两种特别值得亲自验证/深度思考的情形情形一用户可见的界面变更UI change。只靠读代码很难理解某些改动会如何影响用户。如果改动不方便本地打补丁试用可以请开发者给你演示功能。此时评审者亲自验证 CL 行为最有价值。情形二并行编程parallel programming。如果 CL 中涉及可能引发死锁deadlock或竞态条件race condition的并发模型这类问题极难仅靠运行代码发现通常需要开发者和评审者双方都仔细过一遍脑子才能确认没有引入问题。原文档还顺带给出了一个值得引以为戒的推论这正是不该采用可能产生竞态/死锁的并发模型的好理由——它会显著增加代码评审和代码理解的复杂度。复杂度Complexity在每一层追问是否过复杂复杂度要在 CL 的每一层检查单行代码是否过于复杂函数是否过于复杂类是否过于复杂过于复杂通常有两种含义代码读者无法快速理解或者开发者在调用或修改这段代码时很可能引入 bug。特别警惕过度工程over-engineering一种特殊的复杂度是过度工程——开发者把代码做得比需要的更通用或添加了系统当前并不需要的功能。评审者应对此格外警惕并鼓励开发者解决现在确定需要解决的问题而不是解决开发者推测将来可能需要解决的问题。未来的问题应该等它真正到来、你能看到它的实际形态和需求时再去解决。这与此仓库中 小型 CL 的核心理念一个自包含的、只解决一件事的最小改动完全呼应——把需求控制在当下是控制复杂度、保证可评审性的共同前提。测试Tests要测试更要测试的测试评审者应当按变更的实际情况要求单元测试、集成测试或端到端测试。一般情况下测试应该与生产代码在同一个 CL 中提交除非该 CL 属于紧急情况。测试本身也需要被评审原文档有一个非常犀利的提醒测试不会测试自己我们也几乎不会为测试写测试——必须由人来确保测试是有效的。因此评审测试时要问当代码真的坏了时这些测试真的会失败吗如果测试下面的代码发生变化它们会不会开始产生误报false positives每个测试是否做了简单而有用的断言不同测试方法之间的测试职责是否划分得当最后原文档特别强调测试也是需要维护的代码——不要因为测试不在主二进制文件里就接受测试中的复杂度。这一点可以和 小型 CL 中CL 应包含相关测试代码、所有 Google 变更都应有测试的要求互相印证。命名Naming名称是沟通的第一介质检查开发者是否给所有东西起了好名字。一个好名字的评判标准很朴素足够长能完整传达它是什么或做什么又不能太长长到难以阅读。命名是代码可读性的基本面评审时逐项扫过类名、函数名、变量名、参数名往往能快速暴露设计层面的模糊。注释Comments解释为什么而不是复述是什么检查开发者是否用清晰的英文写出了易懂的注释且所有注释是否真的必要。原文档给出的判据非常实用注释在解释某段代码为什么存在时是有用的注释不应该解释代码在做什么——如果代码本身不清楚到需要解释那就应该把代码简化例外情况确实存在正则表达式和复杂算法往往非常受益于它在做什么的说明性注释但绝大多数注释应该承载代码本身不可能包含的信息例如某个决策背后的推理。别忘了看改动之前的注释评审时还值得顺带查看 CL之前就存在的注释也许某个 TODO 现在可以删除了也许有一条注释当初就在告诫不要做这个改动而现在恰好应验。此外原文档明确区分了注释与文档类、模块或函数的文档应该表达一段代码的目的、它的用法以及被使用时的行为——这是两种不同的职责评审时都要覆盖。风格Style风格指南是绝对权威Nit: 用于非强制意见Google 为其主要语言甚至大多数次要语言都制定了风格指南。评审时要确保 CL 遵循相应的风格指南。基于 评审标准 的原则在风格问题上风格指南是绝对权威任何不在风格指南中的纯风格点如空白都属于个人偏好。原文档还给出两条操作性很强的实践想改进某个不在风格指南里的点时用 Nit: 前缀——这向开发者表明这是你认为能改进代码的吹毛求疵点但并非强制。不要仅仅基于个人风格偏好而阻止 CL 提交。这也与 评审标准 中Mentoring指导一节的用法一致纯教育性、不关键的评论都用 Nit: 标明非强制。CL 作者不应把大规模风格改动与其他改动混在一起——否则很难看清 CL 到底改了什么合并merge与回滚rollback都会更复杂。例如想重排整个文件格式就应该单独发一个纯格式化 CL再发另一个包含功能改动的 CL。风格变更独立成 CL这一点与 小型 CL 中重构与功能变更分离、便于理解每次改动的原则一脉相承。一致性Consistency风格指南优先局部一致次之如果现有代码与风格指南不一致怎么办原文档给出了清晰的裁决顺序按代码评审原则风格指南是绝对权威凡风格指南要求的内容CL 必须遵守当风格指南只是推荐而非要求时属于判断题新代码该与推荐一致还是与周边代码一致——偏向遵循风格指南除非局部不一致会造成太大困惑如果没有任何其他规则适用作者应与现有代码保持一致。无论选择哪种方式都要鼓励作者为清理既有代码提交一个 bug 并加上 TODO——这正是防止代码库在无数小改动中逐步退化的兜底机制与评审标准代码库往往通过一次次小的健康度下降而退化的判断完全一致。文档Documentation改动行为就要同步文档如果 CL 改变了用户构建、测试、使用或发布代码的方式就要检查它是否同步更新了相关文档——包括 README、g3doc 页面以及任何生成的参考文档。如果 CL 删除或弃用了代码也要考虑相应文档是否该一并删除。如果文档缺失就直接提出要求。逐行评审Every Line必须看懂每一行但允许有例外一般情况下要查看分配给你评审的每一行代码。数据文件、生成代码或大型数据结构可以偶尔扫读但绝不能扫过一段人工编写的类、函数或代码块就想当然地认为里面的内容没问题。显然有些代码值得比另一些更仔细的审视——这是你的判断——但你至少要确保自己理解所有代码在做什么。读不懂代码怎么办要求澄清而不是硬着头皮审如果代码太难读、拖慢了评审进度你应该告诉开发者并等他们澄清后再继续。原文档给出的理由很有人文关怀Google 雇佣的都是优秀的软件工程师你也是其中之一——如果你看不懂这段代码其他开发者很可能也看不懂。所以当你要求开发者澄清时你实际上也在帮助未来的开发者理解这段代码。不擅长某些领域怎么办确保 CL 上有合格的评审者如果你读懂了代码但自认没资格完成评审的某一部分就要确保 CL 上有具备相应资格的评审者——特别是涉及隐私privacy、安全security、并发concurrency、可访问性accessibility、国际化internationalization等复杂问题时。例外情况只审部分时要在评论中注明当你作为多个评审者之一、被要求只审一部分内容时例如只审某个更大变更中的若干文件或只审某个方面如高层设计、隐私/安全影响原文档的建议是在评论中注明你审查了哪些部分优先采用带评论的 LGTMLGTM with comments 的形式如果你想在确认其他评审者已审完其余部分后再给 LGTM要在评论中明确说明这一预期并在 CL 达到目标状态后快速响应。上下文Context看整体文件更看整个系统代码评审工具通常只展示改动周围的几行代码但评审者常常需要查看整个文件才能确认改动是否合理。原文档举的例子非常生动你可能只看到新增了四行但看完整文件才发现这四行处在一个 50 行的方法里——而那个方法现在真的需要拆分成更小的函数了。把 CL 放到整个系统的语境中还要从系统整体角度思考这个 CL 是在改善系统的代码健康度还是让整个系统变得更复杂、测试更少原文档给出了一条不容妥协的红线不要接受会降低系统代码健康度的 CL。因为大多数系统正是通过无数小改动的累积而变得复杂的所以即使新改动中很小的复杂度也要防微杜渐。这条原则与 评审标准 的代码库通过小步退化警示互为表里是评审者需要长期坚守的立场。肯定优点Good Things评审不只是挑错如果在 CL 中看到不错的东西告诉开发者——尤其是当他们以很棒的方式回应了你某条评论时。代码评审常常只聚焦于错误但同样应该给予鼓励与赞赏。原文档甚至指出在指导mentoring的意义上告诉开发者你做对了什么有时比告诉他们你做错了什么更有价值。这一点在 评审评论的写法 中也有呼应人们从对做得好的事情的强化中学习而不只从还能改进什么中学习——看到开发者清理了混乱的算法、写出了堪称典范的测试覆盖或让你从 CL 中学到了东西都值得评论并且要附上为什么你欣赏它。总结清单一次完整评审应该确认的 14 项原文档最后给出了一份评审自检清单评审者在完成一次代码评审时应确保代码设计良好well-designed功能对代码的用户有益functionality is good for the users of the code任何 UI 变更都合理且观感良好任何并行编程都以安全方式完成代码没有超出必要程度的复杂度开发者没有在实现他们可能需要、但现在并不知道自己需要的未来功能代码有恰当的单元测试测试本身设计良好所有东西的命名都清晰注释清晰有用且大多解释为什么而非是什么代码有恰当的文档一般在 g3doc代码遵循风格指南逐行审查了被分配评审的每一行代码并关注上下文确保自己在改善代码健康度并对开发者做对的好事给予赞赏。实践建议把这份清单嵌入你的评审流程这份看什么清单是 评审者指南 完整文档体系中的一环。与它配套使用的还有评审的整体标准判断批准与否的底层原则、导航一个 CL先看描述→再看主要部分→按顺序看完其余、评审速度一个工作日内的响应上限、带评论的 LGTM、评论的写法礼貌、解释理由、标注严重度以及应对反驳。对侧的 CL 作者指南 则从作者视角提供了 良好 CL 描述、小型 CL 与 处理评审者评论 的配套建议。落地建议将本文的 14 项清单整理成一张可勾选的评审模板每次评审前先对照 评审标准 校准基调再按设计→功能→复杂度→测试→命名→注释→风格→一致性→文档→逐行→上下文的顺序推进最后不要忘了——指出问题的同时真诚地肯定做得好的地方。坚持这套流程代码库的健康度会在无数个小步改进中持续上升评审本身也会因为开发者的快速成长而变得越来越快。继续阅读下一步可以学习 如何在评审中导航一个 CL先看整体、抓主要部分、按序扫完以及 代码评审的速度何时响应、如何给出带评论的 LGTM。赞分享文档教程【免费下载链接】eng-practicesGoogles Engineering Practices documentation项目地址https://gitcode.com/gh_mirrors/en/eng-practices点击查看免费下载相关推荐代码评审该看什么深入解析 Google eng-practices 的 What to Look For in a Code Review代码评审该看什么深入解析 Google eng practices 的 What to Look For in a Code Review 代码评审Code文档教程代码评审Kotlin MultiplatformKMP跨平台架构实战指南从 expect/actual 到发布全流程Kotlin MultiplatformKMP跨平台架构实战指南从 expect/actual 到发布全流程 本篇指南以 kotlin specialis文档教程Google 代码评审实践指南Reviewer 如何做好 Code Revieweng-practicesGoogle 代码评审实践指南Reviewer 如何做好 Code Revieweng practices 导读 本文基于 Google Engineer文档教程代码评审上一篇CAP消息重试机制详解从失败处理到死信队列的完整流程下一篇如何用TradingAgents-CN在30分钟内构建你的第一个智能交易系统创作声明：本文部分内容由AI辅助生成（AIGC），仅供参考