2023年7月29日发(作者：)

代码审查的必要性和最佳实践⽬录不做code review基本⼭就是背着定时炸弹前进：什么时候炸了都不知道。现在不做， 将来就是救⽕队员。好的code review会减少⼤量的不确定的坑：这个就像我们⽇常出门上班会确定门是否关好⼀样代码审查的流程先来简单介绍⼀下常见的代码审查的流程。为了开发某个新特性，或者修复某个特定问题，负责的程序员会从代码库的主分⽀（masterbranch）上⾯建⽴并 check out ⼀个新的分⽀，将⼯作分为⼀次到若⼲次的“代码变更”来提交。这每⼀次的代码变更，都可以组成⼀次代码审查的单元，有的公司把它叫做 CR（Code Change），有的叫做 PR（Pull Request），还有的叫做 CL（Change List），但⽆论叫做什么，它⼀般⾄少包含这么⼏项内容：帮助理解代码的描述，如果有问题单（任务）来跟踪，需要包括相关的问题单号或者问题单链接；实际的代码变更主体，包括实际的代码和配置；测试和结果，根据项⽬的情况，它可以具备不同形式，⽐如单元测试代码，以及⼿⼯测试等其它测试执⾏的结果说明。多数情况下，以上这三项都不可或缺，缺少任何⼀项都会让代码变更失去⼀定可审查的价值。进⾏审查的，⼀般是⼀起⼯作的，对代码涉及变更熟悉的其他程序员。这个“熟悉”，既包括业务，也包括技术，⼆者当中，有⼀项不具备，就很难做好审查⼯作，给出有建设性的审查意见。接下去的交互就在这个代码变更上⾯了，审查者会提出其问题和建议，变更的作者会选择性采纳并改进变更的描述、代码主体以及测试。双⽅思考、争辩，以及妥协，⽬的都是寻求⼀个切合实际且可以改进代码质量的平衡。如果审查的程序员觉得代码没有太多问题，就会盖上⼀个“Approved”或者“Shipped”戳，表⽰认可和通过。这根据项⽬⽽定，⼀般代码变更最少要得到 1~3 个这样的认可，才可以将代码变更合并（merge）到主分⽀。⽽主分⽀的代码，会随着 CI/CD 的流程进⼊⾃动化的测试程序，并部署上线。代码审查的争议在介绍代码审查的好处之前，我想先来谈谈争议。因为我观察到⼤多数的争议，都不是在否认代码审查的好处，⽽是聚焦在不进⾏代码审查的那些“原因” 或者 “借⼝”上，⽽有些讽刺的是，我认为这⾥⾯⼤部分的“原因”，所代表着的因果关系并不成⽴。加班要累死了，完成项⽬都来不及，还做什么代码审查？类似的问题还有，“代码审查拖慢了进度”，“代码审查不利于快速上线”。这是最常见的不做代码审查，或者草草进⾏代码审查的理由了，但是稍稍⼀细想，就会发现这⾥的因果逻辑完全不对。这就像以前国内⼤兴“敏捷”的时候，有好多程序员，甚⾄项⽬经理，觉得因为项⽬时间紧才要实施敏捷，因为可以少写⽂档，少做测试，随意变更需求，可这⾥的因为所以根本是⽜头不对马嘴。我记得知乎上有句流⾏的话叫做，“先问是不是，再问为什么”，这⾥也可以⽤，因为项⽬压⼒⼤就让“不做代码审查”来承担后果，这实在是过于牵强了。项⽬压⼒⼤，时间紧，可以草草做分析，不做设计，直接编码，不做重构、不做测试、不做审查，直接上线，快及⼀时，可是造成的损失，最后总是要有谁来背锅的。这个锅很可能，就是上线后⽆尽的问题，就是恶性循环加班加点地改问题，就是代码⼀个版本⽐⼀个版本烂。当这些问题都焦头烂额，就更不要说团队和程序员的成长了。代码审查太费时间，改来改去⽆⾮是⼀些格式、注释、命名之类不痛不痒的问题。这也是个逻辑不通的论述，虽然这个还⽐前⾯那个稍微好⼀点。只能提出这些“次要问题”，很可能是代码审查的能⼒不够，⽽并⾮代码审查没有价值；或者是代码审查的⼒度不够，只能提出⼀些浅表的问题，这个现象其实更为普遍。前⾯已经介绍过了，⼀是技术，⼆是业务，⼆者缺⼀都⽆法做出⽐较好的审查。在某些特殊情况下，有时候确实不具备完备的代码审查条件，我们现在来分业务、技术⽋缺的情况进⾏讨论。如果团队中有业务达⼈，但是技术能⼒不⾜。⽐如说，新版本使⽤的是 Scala 来实现的，但是团队中没有精通 Scala 的程序员，这个时候可以寻找其它团队有 Scala 经验的程序员来重点进⾏技术层⾯的代码审查，⽽⾃⼰团队则主要关注于业务逻辑层⾯。当然，既然是⾃⼰团队的代码，所⽤到的技术要慢慢补起来。如果团队的成员具备技术能⼒，但是业务不了解。这种情况也可以进⾏将业务和技术分开审查这样的类似处理，但是如果业务相对复杂，可以先开⼀个预审查会，就着代码或者设计⽂档，简单地将业务逻辑介绍和讨论清楚，再进⾏审查。团队的习惯和流程就是不做代码审查，⼤家都是这么过来的。我觉得这也不是⼀个论述不应该做代码审查的正当理由，类似的还有“绩效考评⼜不提代码审查”，以及“我上班、码代码、下班、拿钱，审查代码⼲什么”。⼤家都不做，并不代表不做就是正确的，如果你赞同代码审查的好处和必要性，那么你的思考会告诉你，应该做这件事情，⼤家不做并不是⼀个理由。从来如此,便对吗如果你发现这件事很难推动，你可以尝试去和你的项⽬经理聊⼀聊，或者结合⾃⼰的项⽬以及下⾯会讲到的代码审查的好处论⼀论，看看是不是能说服那些没有意识到代码审查好处的程序员和项⽬经理。当然，这是另外和⼈沟通以及表达⾃⼰观点的事情，如果⼤家都是朴素的⼲活拿钱的观点，没有对于代码质量和个⼈发展更⾼的追求，或者价值观和你相距⼗万⼋千⾥，改变很困难，你就应该好好思考是不是应该选择更好的团队了。代码审查不利于团建，因为经常有程序员因为观点不同在代码审查的时候吵起来。这依然不是⼀个正当理由，这就好像说“因为开车容易出交通事故，所以平时不允许开车”这样荒谬的逻辑⼀样。⾸先，如果有偏执的不愿意合作的程序员，那么不只是代码审查，任何需要沟通和协作的活动都可以把争吵的⼲柴点燃。对于这样的程序员的管理，或者如何和这样的程序员合作，是另外的⼀个话题，但这并不能否认代码审查的必要性。当然，在下⽂讲到实践的部分我会介绍⼀些⼩的技巧，帮助你在代码审查中⼼平⽓和地说服对⽅。其次，有控制的⼀定强度内的争执，未必是坏事。有句话叫做“理越辩越明”，除了能做出尽可能合理的决定以外，在争论的过程中，你还会得到分析、思考、权衡、归纳、表达，乃⾄⼼理这些综合能⼒的锻炼，本来它们就不是很容易得到的机会，我们为什么还要放过呢？代码审查的好处下⾯我们来谈谈代码审查的好处。你可能会想，这有什么可谈的，这好处难道不是发现软件 bug，提⾼代码质量吗？别急，代码审查的好处可远远不⽌这⼀个，我觉得它还⾄少包括下⾯这些好处。代码审查是个⼈和团队提升的最佳途径之⼀。这⾥的学习，既包括技术学习，也包括业务学习。和英语学习⼀样，如果只听 BBC 或者 VOA 的纯正⼝⾳，没有任何语法错误，英⽂反⽽不容易学好，学英⽂就要接触⽣活英语，各种⼝⾳，各种不合标准的习惯⽤法。阅读代码也⼀样，要学习不同的代码风格和实现。在做代码审查的时候，如果不理解代码，是⽆法给出最佳审查的。因此⾃⼰会被迫去仔细阅读代码，弄懂每⼀⾏每⼀个变量，⽽不是给⼀个LGTM（“Looks Good To Me”）了事。代码审查是团队关系建设和扩⼤双⽅影响⼒的有效⽅式。争论是这个过程中必不可少的⼀环，争论除了能加深对于问题和解决⽅法的理解，在不断的反驳和妥协中，也能树⽴影响⼒，建⽴良好的关系。另外值得⼀提的是，代码审查可不是说⾮得给别⼈挑刺⼉，对于做得特别漂亮的地⽅，要赞扬，这也是建⽴良好关系的⼀种途径。从团队合作和交流的⾓度来说，程序员往往缺乏沟通，每个⼈不能只专注于⾃⼰的那⼀份代码默默耕耘，⽽是需要建⽴⾃⼰的影响⼒的，代码审查过程中的交互，就是⼀个不可多得的⽅式。识别出设计的缺陷，找到安全、性能、依赖和兼容性等测试不易发现的问题。代码审查在整个软件⼯程流程中还算早、中期，尽早发现问题就能够尽可能地减少修复问题的成本。⽽且，代码审查能够发现的问题，往往是其它途径不易发现的。因此，从这个⾓度来讲，代码审查要有⽅向性，⽐如主流程和某些重要⽤例，在审查的时候可以要求代码变更的程序员提供单元测试，或者是⼿⼯覆盖的测试结果，这样就可以认定这些分⽀覆盖到的逻辑是正确的，不需要在审查时额外关注。设⽴团队质量标杆的最佳实践⽅式在我经历的团队中，基本上代码审查做得好的，代码质量都⾼。这不见得是程序员的能⼒特别出⾊，⽽是通过代码审查把这个质量的 bar 顶起来了。你可以想象，⼀个对别⼈的代码颇为“挑剔”的⼈，他会对⾃⼰的代码截然相反地糊弄了事，睁⼀只眼闭⼀只眼吗？特别对于刚踏⼊职场的程序员来说，这点尤为重要，要知道⼀个⼈刚⼯作的两三年，对性格、习惯这些关乎职业⽣涯因素的影响是巨⼤的，⼀个好的标杆⽐任何⼝号都有效。代码审查⼯具我在这⾥着重讲讲配置流程中⾮常重要的⼀步，就是配置机器审查和⼈⼯审查配合的⼯作流。代码审查是代码⼊库前质量保证的重要⼀环，所以通常是和 CI ⼯具配合使⽤。最好能够让机器⾃动化地完成关于代码风格、静态检查、单元测试等⼯作，这样可以让审查者把最宝贵的时间投⼊到逻辑、设计等难以⾃动化的问题上。这⾥，我和你分享⼀种最常见的⼯作流。第⼀，将代码提交到本地 Git 仓库或者⽤于审查的远端 Git 服务器的分⽀上；第⼆，把 commit 提交给代码审查⼯具；第三，代码审查⼯具开始进⾏机器审查和⼈⼯审查；第四，如果审查通不过就打回重做，开发者修改后重新提交审查，直到审查通过后代码⼊库。⾄于具体的⼯具集，我这⾥给出两个例⼦：第⼀个例⼦是 Facebook 采⽤的⽅式。他们使⽤ Phabricator 作为代码审查⼯具。同时直接使⽤原⽣的 Git Server 作为代码仓服务器。⽤户将改动提交到 Phabricator，然后进⾏机器和⼈⼯审查。检查通过后，Phabircator 负责将代码推送到 Git Server 上，或者⽤户⼿动将本地改动 Push 到 Git Server 上。这个⼯作流使⽤的是原⽣ Git Server 管理主仓，如果你要使⽤ GitLab 或者 GitHub 也可以。关于机器审查的配置，Phabricator 提供⼤量的钩⼦和插件机制，⾮常⽅便。具体细节你可以参考Phabricator 官⽅⽂档。第⼆个例⼦是使⽤ GitLab、Jenkins 和 SonarQube 进⾏配置。具体使⽤ GitLab 管理代码，代码⼊库后通过钩⼦触发Jenkins，Jenkins 从 GitLab 获取代码，运⾏构建，并通过 Sonar 分析结果。这⾥有⼀篇不错的⽂章供你参考。⼀些 Code Review ⼯具：Crucible：Atlassian 内部代码审查⼯具；Gerrit：Google 开源的 git 代码审查⼯具；GitHub：程序员应该很熟悉了，上⾯的 "Pull Request" 在代码审查这⾥很好⽤；LGTM：可⽤于 GitHub 和 Bitbucket 的 PR 代码安全漏洞和代码质量审查辅助⼯具；Phabricator：Facebook 开源的 git/mercurial/svn 代码审查⼯具；PullRequest：GitHub pull requests 代码审查辅助⼯具；Pull Reminders：GitHub 上有 PR 需要你审核，该插件⾃动通过 Slack 提醒你；Reviewable：基于 GitHub pull requests 的代码审查辅助⼯具；Sider：GitHub ⾃动代码审查辅助⼯具；Upsource：JetBrain 内部部署的 git/mercurial/perforce/svn 代码审查⼯具。

代码审查建议建议⼀：⼩批量——每次 Review 的代码量要少研究发现， 成功的 CR 活动⼀定是⼩规模的。例如，《Modern Code Review：A Case at Google》论⽂介绍说， Google 的 CR 活动中，有 35% 的 CR 仅仅修改了⼀个⽂件，90% 的 CR 修改的⽂件数在 10 个⽂件以内，甚⾄有 10% 的 CR 仅仅修改了1⾏代码。代码量少的好处显⽽易见：修改了哪⾥⾮常清晰，问题也会⼀⽬了然。⼀次推给别⼈ 1000+ ⾏代码，还想得到有价值的 Review，可能性微乎其微。当然，⼀次 Review 它代表的功能应该是有意义的，是完整的，如果不是修复缺陷，这⼀定程度上也对开发者迭代地开发功能的能⼒提出了要求。建议⼆：多批次——Review 要频繁发⽣⼩批量必然导致了多批次。在微软 2013 年的⼀篇论⽂《iExpectations, Outcomes, and Challenges Of Modern Code Review》和前述的 Google 的论⽂中都提到了频繁 Review 的做法。其中，Google 的每周每 Developer 的代码变更中位数是 3 个，每周每 Reviewer的 Review 中位数是 4 个。建议三：找对⼈——合适的 Reviewer谁适合 Review 你的代码？选⼀个和被 Review 的代码毫不相⼲的⼈肯定是不明智的。下⾯列出了⼀些潜在的候选⼈：如果你的组织有 Owner 机制，Owner 应该是合适⼈选和你⼯作在相同上下⽂的同事近期修改过相同代码的同事⽐你更资深的程序员，希望得到他们的专业反馈现在已经有⼀些⼯具，能够根据上下⽂推荐 Reviewer，这也为选择合适的 Reviewer 提供了便利。建议四：快速响应当每次 Review 的粒度不⼤，Review ⼜⽐较频繁时，快速响应才能成为可能，也是必然的要求。在这个数据上，Google 的中位数是 4 ⼩时。这个指标可以成为⼀个较好的参照。建议五：使⽤现代⼯具快速响应、⾼质量的 Review 离不开现代⼯具。现代的 Review ⼯具能⾃动集成进⼯作流，⾼亮变化，甚⾄能⾃动汇总变更。这⽅⾯已经有许多现代的⼯具可以使⽤，还没有选好⼯具的读者，可以⾃⾏搜索。建议六：考虑结对编程当我们提到 “⼩批量、多批次、快速反馈” 的时候，如果有过结对编程经验的同学，马上就会反映过来，结对编程和 CR 何其相似。结对编程，共同编程的两位同学拥有完全相同的上下⽂，不存在上下⽂切换的烦恼，没有缺乏时间的烦恼，不需要借助额外的⼯具，反馈随时随地。事实上，在我的眼中，结对编程才是最好的 Code Review。建议七：综合在线 Review 和线下 Review在线 Review 应该是常态化的⾏为。考虑到 CR 的 “知识传播” 价值，线下 Review 是有益的补充。有经验的团队，会周期或者不定期的组织线下 Review，这样能获得⽐在线 Review 更为⼴泛的知识传播⾯，也能引起更为热烈的讨论和辩论，有助于形成更⾼质量的共识。建议⼋：及时表达肯定，委婉表达意见。只针对代码，不针对⼈。这听起来很简单，都知道对事不对⼈的重要性，但是要⾮常⼩⼼不能违背。审查并不是只提反⾯意见的，在遇到好的实现，不错的想法的时候，可以表⽰肯定，当然这个数量不宜多，要不然适得其反。⾄于表达意见⽅⾯，我来举⼏个例⼦：对于⼀些次要问题，我都会标注这个问题是⼀个 picky 或者 nit 的问题（“挑剔的问题”）。这样的好处在于，明确告知对⽅，我虽然提出了这个问题，但是它没有什么⼤不了的，如果你坚持不改，我也不打算说服你。或者说，我对这个问题持有不同的看法，但是我也并不坚信我的提议更好。使⽤也许、或许、可能、似乎这样表⽰不确定的语⽓词（英⽂中有时可以使⽤虚拟语⽓）。这样的好处是，缓和⾃⼰表达观点的语⽓。⽐如说：“这个地⽅重构⼀下，去掉这个循环，也许会更好。”间接地表达否定。⽐如说，你看到对⽅配置了周期为 60 秒，但是你觉得不对，但⼜不很确定，你可以这样说：“我有⼀个疑问，为什么这⾥要使⽤ 60 秒⽽不是其他值呢？” 对⽅可能会反应过来这个值选取得不够恰当。你看，这个⽅式就是使⽤疑问，⽽⾮直接的否定，这就委婉得多。放上例⼦、讨论的链接，以及其它⼀些辅助材料证明⾃⼰的观点，但是不要直接表述观点，让对⽅来确认这个观点。⽐如说：“下⾯的讨论是关于这个逻辑的另⼀种实现⽅式，不知你觉得如何？”先肯定，再否定。这个我想很多⼈⼀直都在⽤，先摆事实诚恳地说⼀些同意和正⾯的话，然后⽤“不过”、“但是”和“然⽽”之类的将话锋⼀转，说出相反的情况，这样也就在⾔论中⽐较了优劣，意味着这是经过权衡得出的结论。建议九：激励机制：激发主观能动性由于CodeReview本⾝跟⼈的经验或者意识都有很⼤关系，很多时候我们会为调动不起开发同学的积极性⽽烦恼，所以为了让⼤家更好的参与这个活动，我们⼀般都需要制定相应的激励机制。也有⼈说了，如果是leader强制跟考核挂钩，那就不需要这个东东了，嗯，但换个⾓度看，跟考核挂钩难道不是另外⼀种“激励”⽅式吗？激励机制的设⽴有很多种，⼀般来说，都是在定期回顾的基础上根据CodeReview的实际情况对表现积极的同学进⾏⼀定的礼品奖励（选择什么礼品，要看组织的经济状况，哈哈）。参考链接: