9.9 KiB
开源代码评审的十个通用步骤
只要你遵循这些通用流程，代码评审并不可怕。
你是否需要在你还没有完全理解整个项目时就对代码进行评审？抑或你避开了评审，以免让你看起来不知道如何进行。
本篇文章想要告诉你一个更好的方法。代码评审 并不需要你知道所有事情。实际上，就我个人经验而言，这种情况非常普遍。
我还记得作为实习生加入 红帽 的时候，被要求参与代码评审。我们当时采取的是 +1 或 -1 的投票系统，而我在一开始的时候常常踌躇于该如何评审。我发现我会问自己，如果我对于一处改动给予了 +1，而别人却投了 -1，我是不是看起来很蠢？
如果你对一处改动投了 +1，而别人投了 -1，这又意味着什么呢？答案是不意味任何事！你可能只是漏掉了一处别人注意到的细节。这不意味着世界末日。这也是为什么我们会用投票系统。正如同所有开源项目一样，代码合并是一项协同工作。
最近，我接到了太多的代码评审工作，以至于我几乎做不过来。我同时也注意到，参与评审的贡献者数量正在稳步减少。
出于这个原因，我想要写一篇文章阐述我对代码评审的个人观点。在这篇文章里，我会分享一些诀窍与技巧。我将会向你展示几个用来问自己的问题，以及在评审代码时需要注意的一些地方。
代码评审的目的是什么？
你是否曾写过一个非常简单的补丁？你认为它是如此微不足道，不需要审查。或许你直接就合并了它。直到晚些时候，你意识到你犯了个错误，一个明显的或是愚蠢的错误，比如错误的缩进，比如几行重复的代码而不是调用函数（是的，这些都是经验之谈！）。
如果有其他人来审查代码，就会发现这些东西。
代码评审的一个目的便是为你带来一双新的眼睛，从新的视角看待你要尝试解决的问题。这种新的背景也正是为什么代码评审至关重要。
你可能认为你必须是一个语言专家，才能审查别人的代码、项目，或两者。让我来告诉你一个所有代码评审者都想跟你说的秘密吧：大错特错！你并不需要完全理解该项目或者编程语言，就可以为一个改动提供全新的视角。下面，我将向你展示代码评审的通用流程。
代码评审的通用流程
这是我的代码评审流程，拆分成了几个要点。这个流程包含了我会问自己的一些问题，以帮助我专注于代码的变化以及其后果。你不需要严格依照这个顺序来进行评审。如果有任何原因导致你无法执行其中的某一步，跳过那一步就好。
1、理解改动，它想要解决的问题，以及为什么要这么做
为什么需要改动的解释以及任何相关背景都应该被放在 提交 信息里。如果没有，请要求提供，并请投 -1 直到相关信息被提供。
改动想解决的问题需要被解决吗？它是项目应当关注的问题，还是与项目完全无关？
2、你会如何实现解决方案？它会不一样吗？
在这个时候，你应该已经知道代码改动是为了什么。换做是你会怎么做？在进一步对改动进行细节评审前，先思考这个问题。如果你想出了一个不一样的解决方案，并且你认为你的方案更好，在评审中提出来。你不需要投 -1；去问问作者为什么没有往那个方向走，看看这次讨论会把你们带向何方。
3、运行有改动和没有改动的代码
我通常会在代码中设置几个断点，运行代码并检查新代码是如何与其余部分互动的。
如果你无法运行整个代码，试着将带有新代码的函数复制到一个新的本地文件，模拟输入数据，然后运行。这在你不知道怎么运行整个项目，或者无法接触到运行所需的特殊环境时很有帮助。
4、新代码会破坏任何东西吗？
我是说，任何东西。想一想可能的后果。
以一个新的命令行选项为例，它会总是被目标所接受吗？
是否存在这样一种情况，使得新选项无法被接受或是会与其他东西起冲突？
或许新代码是导入了新的东西。那么这个新的库，以及可能的新的依赖关系，能够在老版本或者项目的运行系统中被找到吗？
安全方面呢？新的依赖足够安全吗？你至少可以在网上快速地搜索一下。还有，注意一下控制台日志里的警告。有的时候在同一个库里也可以找到更安全的函数。
5、新代码是否有效？
你刚刚确认了被提出的解决方案大概是正确的。现在该检查代码本身了。你需要关注代码的有效性和必要性。
检查新代码的风格。它与项目的代码风格相匹配吗？任何开源项目都（应该）有一份文档告知（新）贡献者项目所遵循的风格和优秀实践。
比如说，OpenStack 社区的所有项目都有一份 HACKING.rst 文件。你经常也能找到一份新贡献者指南包含所有必须知道的信息。
6、确认所有新增的变量和导入都被使用
你正在评审的代码常常已经过多次迭代，有的时候代码的最终版本与初始版已迥然不同。所以我们很容易忘记一些在历史版本中加入的变量与引用。自动化检测通常会用到 lint 工具，类似 Python 中的 [flake8][12]。
（LCTT 译注：lint 指编程中用来发现代码潜在错误和约束代码风格的工具，起源于 C 语言编程中的静态分析工具 lint。“lint” 本意为衣服上积累的绒毛与灰尘，“lint” 的取名寓意则在于捕捉编程时产生的“绒毛与灰尘”）
（LCTT 校注：我建议，“Lint” 工具可以翻译为 “代码清理” 或 “代码清洁” 工具。）
你可以在不声明新变量的情况下重写代码吗？通常情况下你可以，但问题是这样是否更好。这会带来什么益处吗？我们的目标不是要创造尽可能多的单行代码，而是写出高效且易读的代码。
7、新的函数和方法是否必要？
项目里的别的地方是否存在可以被复用的功能类似的函数？确保避免重新发明轮子以及重新实现已经被定义的逻辑永远都是值得的。
8、有单元测试吗？
如果补丁增加了新的函数或者在函数内添加了新的逻辑，它也应该附带对应的单元测试。新函数的作者总是比别人更适合写该函数的单元测试。
9. 验证重构
如果这次提交对现有代码进行了重构（它可能重命名了某个变量，或者是改变了的变量的作用域，或者是通过加减参数来改变函数的足迹，又或者是删去了某个东西），问一问你自己：
- 这个可以被删除吗？它会影响到稳定分支吗？
- 所有出现的地方都删掉了吗？
你可以利用 grep 命令 来查找。你不会相信有多少次我投 -1 就是因为这个。这是一个任何人都会犯的简单错误，也正因如此任何人都可以发现它。
提交的所有者很容易忽略这些事情，这完全可以理解。我也犯过很多次这种错误。我最终发现问题的根源在于我太急于提出评审，以至于我忘记了对仓库进行整体检查。
除了对项目仓库的检查外，检查其他代码用户也十分必要。如果有别的项目导入了这个项目，它们可能也需要进行重构。在 OpenStack 社区中，我们有对应的工具来查询别的社区项目。
10、项目文档是否需要做出更改？
你可以再一次使用 grep 命令 来检查在项目文档中是否提到了相关的代码改动。用常识来判断这次改动是否需要被收入文档以告知最终用户，还是只是一个不会影响用户体验的内部变化。
额外提示：考虑周到
当你在评审完新代码后提出建议或评论时，要考虑周到，反馈准确，描述详尽。如果有你不理解的地方就发出提问。如果你认为代码存在错误，解释你的理由。记住，如果作者不知道什么地方出了问题，他们就无法修复它。
最后几句
唯一的坏评审是没有评审。通过评审和投票，你提供了你的观点并为此投票。没有人指望你来做出最终决定（除非你是核心维护者），但是投票系统允许你提供你的观点和意见。相信我，补丁所有者会很高兴你这么做了的。
你能想到别的要点来给出好的评审吗？你是否有我不知道的特殊技巧？在评论中分享它们吧！
via: https://opensource.com/article/22/10/code-review
作者：Martin Kopec 选题：lkxed 译者：yzuowei 校对：wxy