Py学习  »  Git

评审GitHub上的3500多个拉取请求,我学到了什么

AI前线 • 1 年前 • 176 次点击  

作者 | Mickaël Andrieu
译者 | 明知山
策划 | 刘燕

我写这篇文章是为了帮助大家更好地完成代码评审工作,因为我觉得这项工作被误解了,或者被认为是无用功。

什么是代码评审?

对于许多人(也许是绝大多数的开发人员或数据人员)来说,是这样定义的:

这项任务就是通过分析代码来检测错误和 bug,可能是功能问题(代码没有做它们应该做的事情)、回归(新代码破坏了已有的行为)、语法错误(遗漏了符号、调用了未定义的变量)或项目合规问题(缩进、命名规范、语言标准)等。

也就是说,评审人员评判同事甚至是外部开发人员(在开源项目中)的工作,并且必须指出其中所有负面的东西。

如果这是你期望从同事那里得到的代码评审,那么你很难找到一个评审志愿者。

在不质疑这个定义的情况下,我发现它其实是有局限性的,甚至还有点问题——我将与你讨论代码审查的其他作用。

代码评审是一种信息的来源

我要求或做的大多数代码评审都是在这种情况下完成的。

例如,在 2019 年的 PrestaShop 项目中,没有哪一份被接受的代码贡献逃过了我的眼睛。

根据 GitHub 的统计,这些占了我贡献的 40%,而在我看来,这占了我工作时间的 60% 到 70%。

为什么会这样?因为我们都知道,不是所有的东西都会被记录下来,不是所有的东西都会被测试到,但必须有人能够维护开发分支上已经接受的所有东西。

说真的,认为贡献代码的人会在需要他们的时候出现,这是一种妄想。在一个大多数开发人员每两年就会换工作的世界里,这种想法太天真了。

在代码评审过程中,我可以问一些关于背景或者为什么要这么做的问题,如果看到了一些脏代码,就要求做出解释。我的评审记录中经常会出现“为了……”,因为当你不得不回头去理解为什么“那个混蛋”会这样写代码时,了解当时的背景会很有帮助……

代码评审是一种沟通工具

在代码评审中,我越来越多地融入了“非开发人员”的意见。我会拿出一段代码,解释它的里里外外,以此来获得他们的想法。

例如,当添加了一个新特性时,我意识到新的实现可能会对另一个特性产生影响。这时,我会告知产品团队的人,向他们解释我的解决方案的优缺点,并请他们做出决定。

理想情况下,我希望所有参与评审的人都能参与进来——架构师确认新的实现不会破坏已有的架构,内容经理对内容进行验证,开发人员检查是否存在代码错误或做出进一步优化,“产品 /QA”经理确认新特性符合预期……有了评审工具,我们可以让每个人都参与到代码评审过程中。

PrestaShop 项目就是这么做的,根据贡献的不同,会有以下这些角色参与:

  • 负责验证国际化的人员;

  • 提供意见的社区成员;

  • 至少一到两个开发人员;

  • 一个来自产品团队的人,因为必须确保产品的一致性;

  • 最后是 QA 团队,如果没有他们,不能接受任何代码!


有7个人参与评审这个特性

很多人围绕着一段代码展开交流,这很好。

代码评审是一种改进代码的工具

当然,这始终是代码评审的目标之一。确切地说,代码总是可以改进的。问题是,什么时候才足够好?

只要代码具有足够的可扩展性,被测试覆盖到,并且不存在明显的 bug 风险,那就很好了。

对于我来说,更重要的是命名、结构和文档——一致性是必不可少的,特别是对于难以理解的大型和老项目来说。

有时候,良好的命名或把代码写成良好命名的函数(并且只有一个职责)可以减少对文档的依赖。

我个人有一个原则——没有被记录的东西就不存在。

我还禁止 PrestaShop 中出现任何带有布尔参数的函数——不出三周,肯定没有人知道这个参数发生了什么变化,更重要的是,如果函数的行为取决于布尔值,就表示它有两种行为。

我通过代码评审改进得越多,就越是要求使用包含较少参数(零到一个)的短函数和具有特定职责的短类。

大型遗留项目的主要问题不是拥有太多的文件,而是很难了解每个文件的作用……

编码标准的重要性

就像 Symfony、API Platform、PrestaShop 或任何其他严肃的项目一样,配置好你需要的检查和修复工具,尽量不要自己制定标准,让工具来处理它们就可以了!

PrestaShop最终使用的代码评审

如果说有什么事会让我恼火,也可能会让所有人恼火,那就是看到一个评审人员浪费了一个半小时在一段代码上,结果只提出说是因为空格、逗号或大括号没有对齐之类的问题 。
如何提高代码评审技能

下面是我在过去几个月里做的 / 理解的一些有积极影响的小事情。

  • 指出错误和你认为做得不错的事情;

  • 在任何情况下都要感谢评审和验证你代码的人;

  • 在任何情况下都要感谢做出贡献的人,即使你拒绝了他们的代码;

  • 说明你贡献的代码的背景!我们说 RTFM(去阅读手册),而不是 RTFC(去阅读代码),是有原因的:代码并不会包含所有的业务逻辑知识,即使你是一个 DDD 成瘾者;

  • 永远不要说“这太愚蠢了……”,如果你必须做出强烈的批评,可以说“这段代码或这个代码块”,你要针对的是工作,而不是人;

  • 我坚持认为善意和幽默是有帮助的;

  • 如果有不清楚的地方,不要犹豫,把代码拿到你的机器上尝试运行。

  • 我越来越经常问我的同事:“你检查过它是否可以正常运行了吗,还是你只是看了下代码而已?”“通常情况下,一些代码需要检查其可能存在的诱导性行为。这不是额外的工作,而是预期工作的一部分;

  • 评审代码比编写代码更有价值。为什么?因为一个代码贡献,只要它不被接受,就没有价值——每周有 20 个贡献,10 个被接受,比每周有 70 个贡献,5 个被接受要好。无论是什么项目,这个规则总是有效的。

原文链接:https://levelup.gitconnected.com/what-ive-learned-reviewing-over-3500-pull-requests-on-github-ff542e35ee96

你也「在看」吗? 👇

Python社区是高质量的Python/Django开发社区
本文地址:http://www.python88.com/topic/148632
 
176 次点击