9.6 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 的取名寓意则在于捕捉编程时产生的“绒毛与灰尘”）
你可以在不声明新变量的情况下重写代码吗？通常情况下你可以，但问题是这样是否更好。这会带来什么益处吗？我们的目标不是多写花式的一行代码，而是写出高效且易读的代码。
7. 新的函数和方法是否必要？
项目里的别的地方是否存在可以被复用的功能类似的函数？确保避免重新发明轮子以及重新实现已经被定义的逻辑永远都是值得的。
8. 有单元测试吗？
如果补丁增加了新的函数或者在函数内添加了新的逻辑，它也应该附带对应的单元测试。新函数的作者总是比别人更适合写该函数的单元测试。
9. 验证重构
如果这次提交对现有代码进行了重构（它可能重命名了某个变量，或者是改变了的变量的作用域，或者是通过加减参数来改变函数的足迹，又或者是删去了某个东西），问一问你自己：
- 这个可以被删除吗？它会影响到稳定分支吗？
- 所有出现的地方都删掉了吗？
你可以利用 grep 命令来查找。你不会相信有多少次我投 -1 就是因为这个。这是一个任何人都会犯的错误，也正因如此任何人都可以发现它。
提交的所有者很容易忽略这些事情，这完全可以理解。我也犯过很多次这种错误。我最终发现问题的根源在于我太急于提出评审，以至于我忘记了对仓库进行整体检查。
除了对项目仓库的检查外，检查其他代码用户也十分必要。如果有别的项目导入了这个项目，它们可能也需要进行重构。在 OpenStack 社区中，我们有对应的工具来查询别的社区项目。
10. 项目文档是否需要做出更改？
你可以再一次使用 grep 命令来检查在项目文档中是否提到了相关的代码改动。用常识来判断这次改动是否需要被收入文档以告知最终用户，还是只是一个不会影响用户体验的内部变化。
额外提示：考虑周到
当你在评审完新代码后提出建议或评论时，要考虑周到，反馈准确，描述详尽。如果有你不理解的地方就发出提问。如果你认为代码存在错误，解释你的理由。记住，作者无法修复他不知道的漏洞。
最后几句
唯一的坏评审是没有评审。通过评审和投票，你提供了你的观点并为此发声。没有人指望你来做出最终决定（除非你是核心维护者），但是投票系统允许你提供你的观点和意见。相信我，补丁所有者会很高兴你这么做了的。
你能想到别的要点来给出好的评审吗？你是否有我不知道的特殊技巧？在评论中分享它们吧！
via: https://opensource.com/article/22/10/code-review
作者：Martin Kopec 选题：lkxed 译者：yzuowei 校对：校对者ID