代码审查:您做错了


今天,许多人在开发中使用代码审查 。 该实践是有用的,必要的。 即使您没有进行审核,您也可能知道它是什么。

市场上有很多工具可以使用现成的使用场景,建议和规则来审查代码。 GitHub,Phabricator,FishEye / Crucible,GitLab,Bitbucket,Upsource-清单不胜枚举。 我们在Badoo一次也与他们合作:在上一篇文章中,我讲述了关于代码审查的故事以及我们如何提出自己的“自行车”( Codeisok解决方案)的发明。

有很多信息,您可以在Google上搜索大量有关代码审查的文章,其中包含真实的示例,实践,方法,这些方法讨论了优劣,差劣,如何做以及如何-不需要,需要考虑什么以及不需要什么,等等。 e。一般而言,主题是“吸骨”。

这就是为什么冰山的另一部分可能不会被注意到的原因。

我希望您能认真对待我将在本文中描述的内容,在某些情况下是故意夸大其词。 像其他任何流程一样,进行审核也需要仔细而刻意的实施,而“我们将与其他所有人(如果有的话)一起做”的方法不仅会失败,甚至会造成伤害。

阅读完此介绍之后,您可能会遇到一个问题:代码审查的复杂性是什么? 重点是琐事。 而且,上面列出的大多数工具都会立即提供流程审查(开箱即用:设置,提交/启动-继续!)。

但是实践表明,一切都不像乍看起来那样明显。 包括由于虚构过程的简单性。 当一切正常运行时,经理就有可能冷静下来并停止陷入流程,将其传递给开发人员。 他们将要么遵循该过程,要么采取其他措施来解决审阅过程旨在解决的问题。 管理者可能不知道出了什么问题,因为它没有设置规则或不施加规则。 对于企业来说,尽快解决问题而不浪费时间进行计划外活动非常重要。

曾几何时,当我在另一家公司工作时,与一位领先的开发人员就代码审查进行的交谈深深地陷入了沉思。 是p1lot 。 也许其中一位读者很熟悉他(p1lot,您好:)。 他目前在Badoo与我们合作,这很棒。

因此,PILOT随后说:“对于我来说,在审查过程中最困难的事情是妥协并进一步跳过已完成且正确工作的任务,即使我知道另一种解决方案,即使我更喜欢我的方法也是如此。” 我认为,这是一个关键思想。 我整篇文章的主要信息都围绕着这种假设。

为什么需要代码审查流程?


贵公司有评论吗? 你做对了吗 我的测试可能会让您怀疑。

您只需要回答三个问题:

  1. 代码审查平均需要多少时间(真空中为球形)?
  2. 您如何最大限度地减少审核时间?
  3. 您如何确定对特定任务的检查是否正确完成?

如果您对某些(或全部)问题没有明确的答案,如果您对答案有疑问,那么我敢于假设某些地方出了问题。

如果您不知道要花多长时间,又不经常将其最小化,那么您将如何计划? 进行了四个小时评审的表演者这次可能没有喝一半茶吗? 是否可以确定任务之间喝茶的时间不会增加? 或者,也许审阅者根本不看代码,而只是单击“代码确定”?

而且,即使真诚地进行了审核,我们如何确保随着代码库和公司的增长,完成任务的时间不会增加? 毕竟,管理人员通常希望流程能够加速而不是减慢速度。

如果没有立即想到第三个问题的答案,那就再问一个是有道理的。 你知道为什么你真的需要这个过程吗? 因为“所有大人物都这么习惯”吗? 也许您根本不需要他?

如果您向潜在客户和开发人员询问什么是“正确的”代码审查,您将对听到的不同感到非常惊讶。 答案可能会有所不同,具体取决于个人经验,信念,个人对事物正确与否的信心,甚至取决于所使用的技术堆栈和开发语言。

还记得现成的代码审查工具,提供他们自己的方法和流程吗? 例如,我想知道这些工具的开发人员将如何回答这个问题。 他们对审核“正确性”的答案是否与您的员工的答案相关? 不知道 有时我可悲地看着某人在家中如何实施审阅工具,一分钟都没有怀疑它们是否必要。 人们要么“展示”,要么报告“他们实施了代码审查,一切都很好。” 最后他们忘了它。


我不想成为盖帽,但尽管如此。 与员工交流时,请注意诸如“因为它已经建立”或“这是最佳做法,每个人都做”之类的答案。 您自己很清楚,您无需了解为什么需要该流程就无需考虑周全。 任何过程都应根据业务和项目的需求以及实际存在的问题和您真正想解决的问题进行调整和实施(如有必要)。 具有良好工程文化的公司中的货物崇拜不属于。

因此,对于经理来说,向人们传达在他的项目中确切需要的“正确”代码审查非常重要。 当然,在此之前,应该为自己制定“正确性”标准,选择正确的工具并建立适当的规则。 并且已经接近控制。

错误代码审查


那么正确的过程是什么? 让我们做对。 下面将有很多我的个人推理,对于那些迫不及待地想知道我在写所有这些内容的人,我建议立即进入“良好代码审查”部分。

对于那些留下来的人,我表示感谢,尽管如此,我还是建议您根据项目的需要,自行决定审查的正确性,并仔细考虑所有内容。 我只是尝试为您提供帮助。

为了在任何过程中为您自己突出重要和必要的事物,从消除有害于工程文化的明显不必要的事物开始可能会很有用。 因此,让我们开始吧。

知识共享


对于“为什么需要代码审查流程?”这个问题 我已经多次听到“ 分享知识 ”的答案。 确实,这是一件有用且必要的事情。 许多人都认为,在团队中建立一个交流知识和专业知识的过程很重要。 但是您确定代码复审适合于此吗?


我已经反复问过人们“知识共享”的含义。 作为回应,我听到了不同的事情。

首先,这是向新来的人(团队或受影响的组件)展示已接受的规则和实践的方法:这是我们这样做的习惯,而我们不这样做,这就是原因,这就是原因。

其次,这是初学者(还是团队或受影响的组件)的新外观,说明它们如何完成普通的工作。 突然他们的工作效率低下了,新来的人会帮助发现这一点吗?

第三,这只是部分代码的介绍。 的确,如果将来审阅者面临更改这部分代码的需求,那么对他来说将更加容易。

让我们仔细阅读所有要点,并尝试评估它们在审核过程中的相关性。

代码审查作为初学者的培训工具


在这种情况下,我们主要讨论的是这样一种情况:给初学者一个任务,他执行任务,然后由经验丰富的团队成员(组件的所有者)指出他犯的错误。 我不认为此过程很重要且必不可少-初学者需要以某种方式显示团队接受的规则。

但是说实话:为时已晚吗? 在让初学者接受代码之前告诉初学者这些规则会更正确吗? 毕竟,错误的代价很高-很少有初学者立即按照您需要的方式工作。 因此,该任务将返回并再次进行修订。 因此,该产品将比其更晚时间提供给用户。

事实证明,在一个持续时间难以评估的过程中,我们添加了另一个过程,其时间成本甚至难以预测。 因此,我们将任务交付给用户所需的时间增加了甚至更多的未知数。

当然,有时通过工作过程对初学者进行培训具有权利。 但是有时我们不考虑已成为习惯的方法的缺点。 在这里犯错不仅容易,而且非常容易。

例如,如果公司没有简化的培训和指导流程,则经理被迫“扔入水中”成为新来者。 同时,后者别无选择:您必须“游泳”或离开公司。 有人真的从这种情况中学习,有人没有站起来。 我认为许多人在其职业生涯中都遇到过类似的问题。 我的信息是,由于这种现象,最宝贵的时间可能无法最佳地度过。 在团队中适应新员工的过程中,它可能不是最佳选择。 仅仅因为没有人能够组织有效的流程来将新员工引入团队,经理认为新人会在代码审查阶段学到所有东西。 但实际上,这是两个不同的过程,非常必要且重要。

当然,即使在最初向初学者解释了规则并且公司有其他培训过程的情况下,也需要以某种方式控制适应初学者的过程。 审核是可以提供帮助的过程。 但是控制和训练是两回事,对吗? 而且,团队的所有成员都需要控制,而不仅仅是初学者。 毕竟,我们都会犯错,忘记协议,并以某种方式影响整个团队拥有的系统部分(在这种情况下为代码)。

可以得出什么结论? 上述过程旨在实现一个不同的目标:不是为了培训,而是为了控制。 而且,不仅对于初学者而言,而且对于整个团队来说,都必须使用相同的控件。

所有这些都可以在以下论文中轻松阐述:“ 必须进行代码审查,以监视所有团队成员对协议和一般规则的遵守情况 。” 在这种情况下,对初学者的培训无非是人为地替代了一个目标,该目标只会使人们感到困惑,并朝着另一个方向发展。

但是即使有了这个似乎正确制定的​​结论,柴火也可能被打破。 代码审查是从已经编写代码开始的阶段。 可能会发生这样的情况,即不遵循通用规则而编写的代码对于自定义通用模式而言非常昂贵。 我们的任务是尽早通知承包商出现问题。 因此,我认为有时候代码审查不是监视通用规则遵守情况的最合适过程。 在许多情况下,合规性检查可以而且应该转移到早期阶段。

例如,您可以检查版本控制系统挂钩中代码的格式(以及安全类的使用,相应文件夹中文件位置的安排,外部依赖项和第三方库的使用等)。 这样可以最大程度地减少代码检查所需的时间。

继续讨论在代码审查过程中培训初学者的话题,让我们作一个类比。 假设您生产某种产品,例如蛋糕。 假设您收到了VIP婚礼的蛋糕订单。 您将成品蛋糕的“审查”委托给新手吗? 您准备好让新的糖果店在这次婚礼上对整个面包店的耻辱或成功负责吗? 或者,也许您希望他在此阶段熟悉团队采用的规则(如何烤蛋糕,准备奶油并坚持在干邑白兰地上放蔓越莓)? 显然,在这个阶段向初学者介绍规则的过程和对他的控制都是相当奇怪的事情:蛋糕已经烤好了。 那么,为什么我们在功能和代码方面采取不同的行动呢? 还是我们推出的功能没有在用户和客户眼中带来耻辱或团队的成功?

您可以反对,说我们不是每天都在为“重要人物”准备任务,并且完全有可能让初学者承担不太紧急或不太重要的任务。 你会是对的。

但是首先,初学者从琐碎的任务中学到了什么,这些琐碎的任务几乎没有代码更改(而且这些更改不在关键位置,并且对于业务而言,由于任务很紧急,因此可能并不那么重要)? 如果他一直鞭打奶油,他怎么能学会烤蛋糕? 当然,需要从简单到复杂的过渡。 但是有必要这样做,仔细控制过程。 初学者需要被教导,而不是自己戒烟。

其次,即使是这种看似有用且正确的方法也可能损害公司。 要理解这一点非常简单,可以使用荒谬的方法使结论尽可能简单和易于理解。

想象一下,我们公开宣布产品经理给我们的任务对于培训新手是必需的。 怎么了 并且与代码审查相同:毕竟,我们将一些简单且非紧急的任务委托给初学者,以便他们学习如何以我们习惯的方式工作。

这最终会导致什么? 一个热心的表演者会忠实地履行自己的职责,并认为自己所做的一切都应尽可能地出色,他将接管学习过程。 他可以将任务设置为进行几种实现,而不是一种实现,以便稍后他可以向学习者展示不同方法的弊端和优势。 他可以进行讲座,比较公司内部及外部使用的方法和最佳实践。 他可以做很多事情来教育初学者。 结果,完成任务所需的时间将增加,因为我们专注于培训 ,而不是去开发

在进行代码审查的情况下,这样一个热心的员工会在评论的评论中展开长时间讨论,讨论某些实现的好处和危险,并使用Stack Overflow发布代码段,表明其他负责人中还有其他想法,并提供学生应阅读的文章和书籍的链接。阅读以了解其实现是不完善的。

这是正常的学习过程,可以存在,但必须正确完成。 而且,将其集成到解决业务问题的过程中并非总是值得的。 因为这是两个不同的过程。 让初学者制作蛋糕的不同成分,让他做出几种选择,让某些东西毁掉然后扔掉。 让经验丰富的人向他展示如何行动和讲述旧食谱。 但是,学习“在生产线上”为客户生产产品的想法不是一个好主意。 而且,如果您已经决定了这一点,那么请用手柄“带领新来者”,不允许他在培训期间干扰生产过程。

如果您的公司不是机构,学校,技术学校或任何其他教育机构,那么培训不是您的直接责任。 该企业已雇用您解决其他问题并取得其他成果。

顺便说一句,这就是为什么我们不向Codeisok添加用于上传图像,键入内容,在注释中突出显示代码的功能的原因,尽管已经有很多此类功能的要求。 我们提倡这样的想法,即代码审查不是个人博客,信使,也不是培训服务。 另外还需要一个工具。 确实,为了表明代码中的缺陷,纯文本注释已绰绰有余。

代码审查是对代码的全新了解


让我们继续前进。 以下是“经验交流”的场景:我们在团队中做某事,遵守内部协议,并且我们的眼睛模糊,然后一个新人来了(来自另一家公司或来自另一个组件)–也许他会看到我们的缺陷工作安排。

这个想法很好,听起来很合理。 确实,如果我们做错了怎么办?

但还是要回到任务的生命周期和代码审查阶段的开始。 为时已晚? 蛋糕已经烤好了,蛋糕上涂了奶油,玫瑰花被粘了。 错误的代价很高。 然后我们发现,在另一家面包店的蛋糕中加入苏打水和柠檬汁,以保持光彩。 那又怎样 怎么办 重塑?

显然不是,因为:1)我们总是没有汽水,结果还不错; 2)因为他们不喜欢苏打水的味道,所以顾客可能会从我们这里而不是从另一个面包店取蛋糕; 3)蛋糕准备好了,婚礼在今晚举行,我们将没有时间烤新蛋糕。

那这为什么呢? 有必要分享经验。 需要焕然一新。 但是,在我看来,处于不同的阶段。 例如, 烤蛋糕之前 ,您可以咨询初学者:“您以前是怎么做的? 为什么这种方法更好?” 而且,可能有必要以新旧方式烘焙几块蛋糕,以便让我们的普通客户尝试并获得他们的意见。

文章或报告中所见的最佳实践的过分实施会损害公司和产品。 所有主要参与者都将苏打粉添加到蛋糕中:“号角”,“轨迹簿”,“ Rile.ru”,“ Yumdeks”。 每个人都这样做。” 那又怎样 因此,Petya是否应立即处理已完成任务的重做?

我肯定不会。 如果最初在解决问题之前,您不同意“蛋糕中会有苏打水”,那么在代码审查阶段要求添加苏打水是非常短视的。 您的企业没有实现“在蛋糕中加苏打水”的目标。 如果您的蛋糕已经卖得很好,那么是否有汽水也没关系。

您需要了解,经验交流是另一个与代码审查无关的过程。 它可能更早,更晚或与此同时发生,但有所不同。 您可以与竞争对手安排互助大师班。 有些人试图窃取一种超级秘密的配方和一种带有穿孔器的鲜奶油搅打技术。 有人试图暗中监视别人的想法,并在他们的帮助下改善他们的流程。 但是,在产品即将准备就绪且转换成本很高的阶段,在工作的传送带上执行此操作很奇怪。

毕竟,我们正在谈论审查阶段。 已经编写了代码复查,可以进行下一步了。 此代码正在等待审阅者单击“代码确定”按钮。 您的客户根本不准备接受这样的事实,即您将与一位新厨师一起准备烤蛋糕,向他展示如何烘烤蛋糕并听取他的批评。

代码审查作为一段代码的介绍


也许这看起来是最合理,最容易理解的部分,许多人都同意这一点。 这里的场景理解如下:一个程序员编写了任务代码,其余的人看着它,研究了它,现在他们知道如何使用它了。 因此,我们降低了总线问题的风险:如果开发人员离开,其他人将能够成功使用他的代码。 而且我们也准备好下次其他人可以“触摸”此代码的事实,在这种情况下,他将已经知道如何使用它。

让我们考虑一下这是否真的按预期工作,以及它是否真的可以帮助人们共享有关代码特定部分的能力和知识。


让我们通过编写代码的人的眼光来看情况。 , ?

, - (, , , ). , ? , . , , , .

, , , , . . , ?

, - . , - . , : , . , , , , .

, . , - ? — . , , .

, , , - . , . , , . , , , . . , - : , , .

? , , — . .

best practices « -» « , ». 那又怎样 Best practices , , . , ? . , - , .

best practices, , . . — - , / — . « », « », « — , DI». ? ?

, , . , , — , .

, -, «», , - . ? - , , , . , . , .

-, « » «» , . -, . , , - , : ? . , , ( : , ).

, , . , , ( , ?).

— . , , . . ? bus factor , , - , .

, , , : , ?

— , , ?
— .
— ?
— .

. , ? , , , ? , , , — ?

, , , , , ?

, , , . , , .

, . , , , ( ), , ( , , , — , . .). , .

— , , , . ( , , . .) . — — .

Code review


« ?» . , , .

: - , , - , , . git blame , , , , , .

, , — , , , ( ) — . , , . . , ?


. ?

, , , . , , , . , - ( , ). , , — , , , .

, , . ? , .

, , , . , . , ? .

, , — , . ( )?

« - - » . , . .

: . .


, , . , . , .

, , — . , .

— . , . , . , .

, , :


? , , ? ? , , ? - , ? ? ? 依此类推。

, . , . , .

, , . , , , . , , , .

, best practices, . , , , , . . , , . , .

,


. , , . - , , - .

, , . , API- . , bus factor, .

, . .


- ? , . , . null , « ». .

, . , ? ?

-


检查的这一阶段也非常重要,因为它旨在改善臭名昭著的代码支持

许多人理解术语“代码可维护性”的含义,但并不是每个人都可以解释它的含义。 如果是这样,那么如何将任务设置给团队以维持这种可维护性呢? 因此,我相信,一个考虑如何测试他的代码,更不用说测试自己的代码并编写自动测试以使测试自动化的开发人员,自然会努力确保他的代码满足大多数代码可维护性标准。 的确,没有这个,编写自动测试几乎是不可能的。

任何代码更改的一般建议也可以是正确分解任务 。 从构思到生产的所有流程管道中传递的代码量越少,该代码就越容易检查合规性,进行测试,与其他任务的代码组合并上载。 代码审查也不例外。 在许多文件中包含大量已更改行的任务很难阅读和理解(并牢记依赖项)。 修复错误的风险和成本可能很高。

结论


实际上,仅此而已。 这四点正是在审查阶段需要检查的内容。 它们易于形式化,易于传达给表演者,这意味着制定验证标准和预测持续时间并不困难。 自动化和其他方法可以最大程度地减少审核时间。

最后,我将提供更多提示。

重要的是要记住,修改的任何代码返回都不会以最佳方式影响实现的质量。 即使每个人都是善意的。 在代码审查的情况下,它的工作方式是这样的:开发人员纠正了代码,但没有像第一次迭代中那样彻底地对其进行测试(为什么?毕竟,他已经检查了所有内容并进行了少许更改)。 经验表明,在根据代码检查结果进行了更正的那些地方,大多数情况恰恰变成了错误。 这就是人类心理学的原理。

在本文的开头,我建议您回答有关审阅的必要性和正确性的三个问题。 现在,您有了一种算法来帮助您找到答案。 知道了控制步骤并考虑了任务的大小,很可能可以计划代码检查所需的时间,每次都试图减少它。

实施任何流程,重要的是要记住我们为自己设定的目标和我们要解决的问题。 然后,将谷粒和谷壳中的谷物分开并制定可评估的标准以评估有效性将变得更加容易。 您需要为您自己和您的团队制定它们,这也迫切需要解决已出现的问题,但是可以以自己的方式理解它们。

还有一点很重要:对于一家公司而言公平的那些流程,规则和价值可能没有那么有用,而在另一家公司中可以工作。 我所描述的正确审核的示例在我们公司中有效。 我们促进快速开发,频繁发布,并审查每个任务。 考虑一下这对您的项目有多大的适用性,并且审查是那些不能碰到的过程之一。

但是,决定权当然仍在您手中。

Source: https://habr.com/ru/post/zh-CN413965/


All Articles