评审GitHub上的3500多个拉取请求，我学到了什么 作者 | 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 你也「在看」吗？ ??