真人代码审查

真人代码审查

原文:https://medium.com/hackernoon/code-review-for-real-people-261e3d9124ba

“同行代码评审是你能做的改进代码的最重要的事情”——杰夫·阿特伍德

基本原理

代码评审是那些为了获得大的长期收益而招致小的短期成本的软件规程之一。但是代码评审的积极效果不仅仅是被推迟了,它们只有在评审做得好的时候才会显现出来。然而,负面影响是直接的,而且是确定无疑的:这会消耗开发人员宝贵的时间。

硬币的另一面是糟糕的代码质量。允许质量下滑会产生小的短期收益,但会招致大的长期成本。通过兑现短期储蓄,“把这个拿出去”的愿望增加了把下一件东西拿出去的成本。代码可读性更差,修复 bug 的成本更高,团队作为一个整体逐渐变得不适合在代码库中工作。

由于短期和长期权衡之间的这种紧张关系,毫不奇怪,虽然代码审查是好的,但它没有更经常地进行。解决方案是理解是什么让代码评审工作得很好,然后采用一种实用的方法,在最小化短期成本的同时最大化长期收益。行业研究和我的个人经验证实了 Atwood 的观点,即同行代码评审是实现成本/收益平衡的最佳方式。

同行代码评审是这样一个过程,其中开发人员的同行在不超过 1 小时内单独评审不超过 400 行代码,期望每小时留下大约 15 条评论。

这是我目前对术语“同行代码评审”的工作定义,也就是我在下文中所说的“代码评审”或仅仅是“评审”的意思我稍后会更详细地介绍,但是以这种方式审查代码的主要好处是它有助于:

  • 确保代码可读(不可读的代码是不可维护的代码)
  • 在最便宜的时候捕捉虫子
  • 在整个团队中传播代码库的知识
  • 让每个人接触不同的方法

怎么做

一般过程

首先提交一个拉取请求。尽可能地将更改数量控制在 400 行或更少。确保 PR 对已更改的内容有很好的描述,并且/或者有一个具体内容的列表供评审者查看。Github 有一篇关于如何编写完美的拉取请求的精彩博文。

接下来,让你的一个或多个同事知道 PR 已经准备好接受审查了。在几个小时内,他们中至少有一个人应该能够找到时间单独通读这些更改。(避免团队审查代码的诱惑。smart bear的一项研究表明,这是浪费时间,几乎没有任何结果。)对于大约 100 行的小改动,预计他们需要大约 15 分钟,并留下 4-5 条评论。不要一次复习超过 1 小时。对于较大的 PRs,将评审分成多个部分。

注释代表了从关于功能的问题到在代码中发现的实际逻辑错误的任何东西。使用常见缺陷清单有助于了解要查找什么。问题并不一定意味着需要进行修改,但它们可能表明作者需要解决的可读性问题。

当回答完所有问题并做出任何必要的更改后,可以合并 PR。

预期

下面的期望来自于的大约 2500 次代码审查会议。这些绝不是硬性规定,但肯定会严重影响审核过程。

  • 审查中的 LOC 应低于 200,不超过 400。任何更大的内容都会淹没评审者,缺陷也不会被发现。
  • 小于 300 LOC/小时的检查率会产生最佳的缺陷检测。500 以下的费率还是不错的;如果比这更快,预计会错过很大比例的缺陷。
  • 准备带有注释和解释的综述的作者比那些没有的作者有更少的缺陷。我们推测原因是作者被迫自我审查代码。
  • 总复习时间应少于 60 分钟,不超过 90 分钟。此后,缺陷检测率直线下降。
  • 预计缺陷率大约每小时 15 个。只有在低于 175 LOC 的情况下才能更高。
  • 如果任其自生自灭,即使作者、评审者、文件和评审规模相似,评审者的检查率也会大相径庭。

如何真正做到这一点

好,那么你知道为什么你需要它,以及如何去做它。但是你如何确保你的实际上是审查代码呢?

分配时间

从实习生到架构师,每个开发人员每周应该分配 3-5 个小时进行代码审查。如果你不故意留出这段时间,你会陷入“我们知道我们应该审查代码,但是没有人有时间”的模式

这个时间可能会分布在几天内,但不一定需要平均分布。灵活性是关键。理想情况下,评审应该在被请求的几个小时内进行。一个严格的时间表(如下午 2-3 点,周一,周三和周五)是可以预见的,但也会推迟审查。如果在给定的一周内不需要全部分配,那也没关系,开发人员可以利用这段时间从事他们的其他计划。但是时间是可以利用的,团队之外的人不应该对时间是如何被利用的感到惊讶。

团队领导的责任不仅仅是管理项目,还包括项目涉众的期望。在计划会议期间,应该清楚地传达和说明代码评审的时间分配。尽管这样做很有诱惑力,但是不应该为了提高项目速度而占用这些时间。说真的,别这样!编写大量的坏代码(相对于少量的好代码)可能会带来短期的提升,但这总是以降低速度为代价的。

查看拉取请求

在变更被集成回主线代码的时候评审代码。总的来说,我更喜欢以一种与工具无关的方式推荐流程,但是 Github 的拉请求和代码审查功能太强大了,不能不提。

对于那些没有使用过的人来说,Github 的 Pull Request 是一个特性,它允许开发人员在分支被合并回主线代码之前查看和讨论分支上的更改。大多数基于 git 的平台都提供这种类型的功能,即使它们可能有不同的名字。为简单起见,我将把它们称为“拉请求”或“PRs”

您不必等到代码“完成”后再创建拉请求。如果您想在开发中期审查代码,创建一个 PR,然后在标题中添加一个“不要合并”标签、标记或注释。在您对实现投入太多之前,这是获得更大的体系结构变更的早期反馈的极好方法。在这种情况下,要清楚你到底在寻找什么样的反馈。如果代码不完整,你的同事的目光会被吸引到未完成的部分,除非你特别想让他们发表意见。

同行代码评审应该单独进行。绝大多数研究表明,审查代码最有效的方法是自己查看,而不是在更大的多审查者环境中查看。SmartBear 的研究表明,个人会在审查的代码中发现高达 85%的缺陷,而作为一个群体进行审查只会发现 4%。我个人认为这是疯狂的!也就是说,要求多人进行审查是完全合理的。

任何人都应该被邀请去评审别人的代码。高级开发人员审查初级开发人员的代码是一个明显的例子,但是初级开发人员将通过审查他们的代码向高级开发人员学习。阅读每个人的代码是在团队中传播项目知识的最好方式之一。如果你只是为了寻找 bug 而阅读初级开发人员的代码,那你就错过了。阅读更高级的开发人员编写的代码是学习新事物的好方法。一般来说,阅读代码是确保代码可读的好方法。即使您无权批准或整合变更,您的反馈也是有用的。

最后,尽量将拉取请求保持在合理的规模。研究表明,一个评审者发现缺陷的能力在大约 1 小时和 1 小时后会显著下降。根据经验,预计每小时可以完成大约 400 行代码。这意味着您可以一次审查 400 行变更以下的任何 PRs。保持 PRs 如此之小的一个方法是在特性完全完成之前提交并合并代码。如果你最终有一个拉式请求,你不能保持在 400 行以下,在多个回顾会议中回顾它。

你不需要复习所有的东西

代码评审不仅仅是在代码编写完成后发现缺陷。研究表明,当定期进行代码审查时,开发人员会写出更好的代码。发生这种情况有几个原因。

首先,您审查的代码越多,您自然会在将您自己的代码发送给其他人之前审查它。其次,“老大哥”效应加剧了这一问题。当你认为有人可能会审查你的代码时,你会写出更好的代码,即使审查根本不会发生。即使只有四分之一的减贫战略得到审查,也会产生这种影响。

使用清单

我不想在这里贴一个清单。只要在网上搜索一下,就能找到很多非常好的书。此外,随着项目和人员的增加和减少,您团队的清单应该随着时间的推移而变化。

从针对项目语言和架构的合理清单开始。随着时间的推移,定期检查您的代码评审注释,并找到最常见的缺陷类型。在列表中添加对这些项目的检查。如果已经存在,将该项目移到更突出的位置。用同样的方法检查你的清单。如果清单中的某个项目从未导致任何发现的缺陷,那么删除它。

每个团队或项目维护他们自己的清单是完全合理的。不要试图在一个漂亮干巴巴的文档上节省时间,因为它充满了不相关的内容,对任何人来说都没有用武之地。此外,不要阻止个人使用他们自己的宠物清单。如果你有能帮助你发现缺陷的东西,那就用它。工具越多越好!

使用清单有助于防止“LGTM”👍”从合并前拉请求上留下的唯一注释。使用清单来审查代码的一种方法是从清单上的一项开始,然后只寻找代码中的那些缺陷。然后移动到下一个项目,重复这个过程。

不让一些事情发生

不要浪费时间去寻找可以被机器检查的东西。使用 linter、StyleCop 或任何可以自动识别空白、嵌套词条、长行、大写规则,💩用作变量名…你明白了。将这些检查添加到您的 CI 过程中,以便当样式违规进入代码库时,您的团队可以通过失败的构建立即得到通知。让人类找到机器找不到的东西。

此外,不要要求不引用某种行业标准或预先定义的团队指导方针的变更。不要用代码评审来争论制表符和空格,如果你的指导方针需要制表符,就用它来要求制表符。如果你不同意这些指导方针,一定要开始对话进行修改,但是要在代码审查过程之外进行。

你要求的任何其他改变都将基于个人喜好。这些完全可以提出来,但是在你的评论中要清楚。如果你认为一个switch会比一个带锁链的if / else工作得更好,说一些类似“考虑在这里使用一个switch”的话。我个人认为这会让这篇文章更具可读性。”如果原作者认为不应该改,随它去

最后的想法

在代码评审上投入时间的最终目的是通过做出更好的代码和更好的编码员来省钱。这篇文章是我个人研究和经验的产物,但是如果它不能帮助你消除审查代码的障碍,那么请跳过那些愚蠢的部分,使用你能使用的。如果您想阅读对我的代码审查哲学影响最大的内容,请查看以下资源。

资料来源和进一步阅读


本站为非盈利网站,作品由网友提供上传,如无意中有侵犯您的版权,请联系删除