被认为有害的代码审查
被认为有害的代码审查
原文:https://medium.com/hackernoon/code-reviews-considered-harmful-a4cc9b2323dc
6 个迹象表明你的代码评审是有毒的,正在毒害你的团队

我注意到在过去 3 年左右的时间里,代码评审越来越流行。自从我 2003 年进入这个行业以来,我一直以一种随意的方式参与代码评审。那时你会打印出一些代码并用红笔标记出来。如今,像 Stash 和 Crucible 这样的工具使得代码评审变得极其容易。让代码评审成为拉式请求的一部分确实推动了我们在这方面的进步。
警告:这都是浅见,继续阅读风险自担!我想每个人都会同意这是一件“好”事情。我注意到一些组织和团队已经培养了真正有害的代码评审过程。如此糟糕,赶走了优秀的开发者。为了说明这一点,我将模仿一条代码审查快乐路径和一条代码审查有害路径。
代码评审快乐之路
你刚加入一个团队或公司。也许你是新手,或者你已经在这个行业干了几年了。无论如何,你完成几个功能,提出一些拉请求,设置你的领导或老年人作为审查者。他们审查你的代码,在这个过程中,你可以感觉到他们在寻找什么。你采纳了他们的反馈,你的评论就变成了一种形式。您拨入并开始生产特性和代码。偶尔会发现并修复错误、逻辑错误或可能的安全漏洞。
作为快乐之路的一部分,也许他们已经将 linters 和 findbug 实用程序整合到构建中,以加强诸如组格式标准(例如制表符和空格)之类的东西。也许有一个编码标准文档,讨论命名约定和适当的抽象级别。太好了!
快乐之路的主要特点是:每个人都很开心,代码很好,团队也很有效率。这是每个人都想要的。团队抓住了代码评审的精神:提高软件质量。
代码审查有毒路径
作为一个新团队的新开发人员,与快乐之路不同,在您最初的几次拉请求和代码审查之后,您不会达到本地标准的舒适水平。你已经结合了先前的反馈,并且每一次代码评审都提出了当天另一个评审者的挑剔。在构建中没有林挺或其他强制措施,也没有编码标准,所以你没有什么可遵循的。评论者 Sally 更喜欢名词动词命名,评论者 Rajesh 更喜欢动词名词。有一天 Sally 决定所有带有 Assert.assertAAAA()的 JUnits 都应该有正布尔逻辑作为参数,所以现在有些是 Assert.assertTrue(),有些是 Assert.assertFalse()。这听起来可能很荒谬,但我曾经见过。
有毒路径的主要特征是没有人高兴,发展速度缓慢爬行。代码审查需要很长时间,团队交付特性的速度很慢。交付的软件有明显的缺陷。就我个人而言,我已经退出了允许这种行为的团队和经理。
警告标志
花几分钟时间来检查你的团队代码评审实践的健康状况。如果你的代码审查表现出以下 6 个品质中的任何一个,你可能有多个问题,这些问题正在消耗你的开发速度和开发人员的忠诚度。
- 争论格式问题。像制表符对空格和括号放置在同一行或下一行这样的事情应该由构建来解决和执行。
- 评论者竞争最聪明的一行或更多的抽象和间接层。代码应该是可维护的和适当的抽象层次。对我来说,这意味着最低层次的抽象。这显然是一个有争议的问题。说出来!
- 争论变量、方法、类或包的名字。决定一种方法并坚持下去。
- 审阅者需要一些奇怪的东西,比如上面的 Assert.assertTrue 示例。理想情况下,所有的标准都来自行业标准规范,比如坚实的原则。如果一个评审者不能用那些术语表达一些东西,那么它就不应该是一个编码标准。
- 先前未陈述的需求被呈现为代码评审所需的变更。遗漏的需求应该是下一次 sprint 的增强。
- 审查者要求对不属于审查范围的代码进行修改。代码美化和重构应该也是下一个 sprint 的任务。
简而言之,如果一个编码标准不能被机械地强制执行(例如,在构建中有一个标记),引用一个行业标准,或者引用一个集团范围的标准文档,那么它应该被禁止在代码审查中。这样我们就可以集中精力寻找错误、逻辑错误和安全漏洞。



