你是否曾经发现自己需要进行代码审查，但并不完全了解项目？也许你没有审查，以避免看起来你不知道自己在做什么。
本文向你保证，有一种更好的方法。你不需要知道一切才能提供代码审查。事实上，根据我的经验，这很常见。
我记得当我加入红帽公司实习时，被要求协助进行代码审查。我们使用 +1 或 -1 的投票系统，起初我很犹豫要参与。我发现自己会问，如果我对一个变更投了 +1 票，但后来其他人投了 -1 票，我会不会显得很傻？
如果有人对你投了 +1 票的变更投了 -1 票会发生什么？答案是什么都不会发生！你可能错过了其他人注意到的细节。这不是世界末日。这就是我们拥有这个投票系统的原因。与开源的其余部分一样，合并代码是一项协作工作。
最近，我被大量的代码审查淹没，几乎跟不上进度。我还注意到，进行这些审查的贡献者数量稳步下降。
因此，我正在写下我对编写代码审查的看法。在本文中，我将分享一些有用的技巧和窍门。我将向你展示你在进行代码审查时应该问自己的一些问题，以及一些应该寻找的东西。
代码审查的目的是什么？
你是否曾经编写过一个非常简单的补丁？你认为它是如此微不足道，以至于不需要审查？也许你直接合并了它。后来，结果发现有一个错误，一些明显或愚蠢的错误，比如错误的缩进或一些重复的代码行而不是一个函数调用（是的，我说的是我的经验！）
别人的代码审查会发现这些问题。
代码审查的目的是引入一双新的眼睛，以新的视角看待你试图解决的问题。这种新的背景正是代码审查至关重要的原因。
你可能认为你必须是该语言、项目或两者的专家才能审查别人的代码。这里有一个所有代码审查员都希望你了解的秘密：这是错误的！你不需要完全了解项目或语言来对变更提供新的视角。代码审查有一个通用的流程。
代码审查的通用流程
这是我的代码审查流程，分为几个要点。该流程提供了我问自己的问题，以帮助我专注于代码变更及其后果。你不需要按这个特定顺序进行。如果由于任何原因你无法执行某个步骤，只需移动到另一个步骤即可。
1. 了解变更、它试图解决的问题以及原因
对为什么需要变更的解释以及任何相关背景信息应该在提交消息中。如果不是，请请求它，并且可以随意投 -1 票，直到提供为止。
这是一个需要解决的问题吗？这是项目应该关注的事情吗，还是完全超出范围？
2. 你将如何实施该解决方案？会有所不同吗？
此时，你了解了代码变更的内容。你会怎么做？在详细审查变更之前考虑一下。如果你想到的解决方案与你正在审查的解决方案不同，并且你认为它更好，请在审查中提出这一点。你不需要投 -1 票；只需询问作者为什么没有朝这个方向发展，并观察讨论如何发展。
3. 运行包含和不包含变更的代码
我通常会在代码中放入几个断点，运行它，并检查新代码如何与其余代码交互。
如果你无法运行整个代码，请尝试将包含新代码的函数复制到新的本地文件，模拟输入数据，然后运行该文件。当你不了解如何运行整个项目，或者需要你无法访问的特定环境时，这会很有帮助。
4. 新代码会破坏任何东西吗？
我的意思是，真的任何东西。考虑后果。
对于新的命令行选项，目标是否总是接受它？
是否会出现选项不被接受或可能与某些东西冲突的情况？
也许这是一个新的导入。新的库，以及可能的新依赖项，是否在您为项目发布的旧版本或系统中可用？
安全性怎么样？新的依赖项是否可以安全使用？你至少可以做的是快速进行互联网搜索以找出答案。另外，请查找控制台日志中的警告。有时，在同一个库中存在更安全的方法。
5. 代码是否有效？
你已经确定建议的解决方案可能是正确的。现在是检查代码本身、其有效性和必要性的时候了。
检查新代码的样式。它是否与项目的样式匹配？任何开源项目都有（或者应该有）一份文档，告知（新的）贡献者项目遵循的样式和良好实践。
例如，OpenStack 社区中的每个项目都有一个 HACKING.rst 文件。通常还有一个 新贡献者指南，其中包含所有必备信息。
6. 检查所有新变量和导入是否都被使用
通常，你正在审查的代码已经经历了多次迭代，有时最终版本与开始时有很大不同。很容易忘记在新代码的先前版本中需要的导入或新变量。自动化通常使用 linting 工具（例如 Python 代码中的 flake8）来检查这些内容。
你能否在不声明新变量的情况下重写代码？嗯，通常可以，但问题是那样是否更好。它带来任何好处吗？目标不是尽可能地创建单行代码。目标是编写高效且易于阅读的代码。
7. 新函数或方法是否必要？
项目中是否有一个类似的函数可以在某处重复使用？始终值得帮助避免重新发明轮子并重新实现已经定义的逻辑。
8. 是否有单元测试？
如果补丁在函数中添加了新函数或新逻辑，它也应该包含该函数的新单元测试。当新函数的作者也为其编写单元测试时，总是更好。
9. 验证重构
如果提交重构了现有代码（它重命名变量、更改变量作用域、通过添加或删除参数来更改函数的 footprint 或删除某些内容），请问自己
- 这可以删除吗？会影响稳定分支吗？
- 所有出现的地方都被删除了吗？
你可以使用 grep 命令 来找出答案。你不会相信我因为这个原因而投 -1 票的次数有多少。这是一个任何人都可以犯的简单错误，但这也意味着任何人都可以发现它。
提交的所有者很容易忽略这些事情，这完全可以理解。我也发生过很多次了。我终于弄清楚了我一直在修复的问题的根源，所以我急于提出审查，然后我忘记检查整个 repo。
除了项目的存储库之外，有时还需要检查其他代码使用者。如果其他一些项目导入了这个项目，它们也可能需要重构。在 OpenStack 社区中，我们有一个工具可以搜索每个社区项目。
10. 项目文档是否需要修改？
同样，你可以使用 grep 命令 来检查项目文档是否提到了任何与代码变更相关的内容。运用常识来确定是否需要为最终用户记录更改，或者它只是不影响用户体验的内部更改。
额外提示：考虑周到
在你审查了新代码之后，如果你提出建议或评论某些内容，请考虑周到、精确和具有描述性。如果你不理解某些内容，请提出问题。如果你认为代码是错误的，请解释你为什么这么认为。请记住，如果作者不知道哪里坏了，他们就无法修复它。
总结
唯一不好的审查就是没有审查。通过审查和投票，你提供了你的观点并仅为其投票。没有人期望你给出最终的肯定或否定（除非你是核心维护者！），但投票系统允许你提供你的观点和意见。补丁所有者会很高兴你这样做了，相信我。
你能想到一个好的审查的其他步骤吗？你是否有任何与我不同的特殊技术？请在评论中告诉我们！
评论已关闭。