健全的办公环境，并有代码审查准则

在我的新团队里，我们正在制定一些指导方针、规则和流程改进方案。为什么我们认为这些如此重要？如果所有事项都记录在案，新成员就能更容易地上手工作，创造价值。这也能降低所有人犯错的可能性，并消除很多争论的可能性。我们都知道，争论永远赢不了，所以我们应该尽一切可能避免争论。

想要更详细地了解指南的重要性，请查看这篇文章，顺便说一句，我很快还会再谈到这篇文章。

这次我将重点讲解代码审查及其相关准则。

代码审查的目的

审查拉取请求是一项重要且敏感的任务。在我看来，它至少与编写代码同等重要。此外，审查他人的代码不仅仅是一项技术任务，也是一项人性化的任务。这正是它微妙之处的根源所在。

那么，让我先从最重要的规则开始，这条规则在你开始审查拉取请求或打开收到的审查意见时都应该牢记在心：

评论不应涉及个人恩怨。不应评论作者或审稿人。评论必须始终围绕代码本身！

代码审查的目的是为了改进代码，在合并和交付之前发现错误，此外，还要提高给定代码库的可维护性。

代码审查中需要检查的项目

代码审查是一项艰巨的任务，实际上涉及范围非常广。我的老板们认为我是一名优秀的代码审查员，但我仍然觉得我的效率还有很大的提升空间。我认为，在大多数情况下，遵循检查清单会大有帮助。

显然，有些清单和/或任务会因语言而异。然而，好的复习之所以有效，大多数情况下都遵循相同的概念，而与所使用的语言无关。

这些清单主要是为了给你一些启发，远非完整。你可以随意使用、更新、个性化它们，或者仅仅从中汲取灵感，创造出全新的清单。

我认为审阅者不应该全部使用清单，或许只需要用到其中几个即可。但如果你们有单独的清单，就很容易分担任务。

请注意，并非所有检查清单都适用于所有代码审查。如果拉取请求只是一个很小的错误修复，例如修正条件判断中的一个差一错误，则无需检查整个域的设计。

完整流程检查清单

本文重点介绍拉取请求的一些基本特性。我没有添加新的内容，但您应该确保新提交不会破坏编译或测试。您的持续集成流水线应该会处理这些问题，但以防万一，请务必注意。否则，请检查以下内容。

• 是否新增了单元测试/回归测试？
• 是否有新的编译器警告？
• 这种改变在功能上是否合理？
• 依赖项很多吗？
• 提交信息是否清晰？
• ...

SOLID（面向对象设计）原则清单

为了验证设计的合理性，有必要仔细检查SOLID原则。最好将这些原则展开成子列表，以便更好地验证每个原则。

• 单一职责原则
• 开/闭原则
• 里氏替换原则
• 界面隔离原理
• 依赖倒置原理
• ...

安全检查清单

你的应用程序可能属于安全关键型应用，也可能不属于。但一旦它被黑客攻击一次，或者因为一些错误的输入而失败，它就变成了安全关键型应用……这份清单如果可用，应该很大程度上取决于编程语言，我这里提供一个 C++ 的清单。我的一个同事主要根据他在 2018 年 NDC 安全大会上关于安全编程实践的演讲整理了这份清单。

• 外部输入是否得到正确处理？
• 是否使用 C 风格的接口？
• 是否new多余地使用了运算符来代替栈分配？
• 是否存在大量（容易出错的）尺寸计算？
• 指针的使用频率高吗？
• shared_ptrs 使用频繁吗？
• 有讨论串吗？
• ...

测试最佳实践清单

我希望大家都认同测试是开发人员工作的一部分，如果要讨论测试，应该讨论的是不同的测试方法，而不是是否应该进行测试。坏消息是，没有一种万能的测试方法——但我仍然建议你遵循测试驱动开发（TDD）的流程。好消息是，在项目中，大家通常至少对应该做什么有共识。如果没有共识，那就应该积极参与，倡导测试，收集相关的文章和研究，并努力说服大家。你会因此赢得更多尊重。
以下几点需要澄清，以明确测试的相关内容：

• 单元测试够吗？
• 非回归检验是否足够？
• 测试只测试一件事吗？
• 它们有断言吗？（一个测试可能包含多个断言，但从逻辑上讲，它们仍然只断言一件事。）
• 它们可读吗？
• 日期是如何使用的？（固定日期与自动生成的日期）
• ...

代码可读性检查表

我们——开发者——都是作者。如果我们精益求精，我们的代码读起来就像散文一样优美。我并不是说整个代码库都必须达到这种水平，但我们应该朝着这个方向努力。代码审查员在这里肩负着重大的责任。如果你正在阅读一个拉取请求，请思考以下问题：

• 名字有意义吗？
• 类/函数是否足够小？
• 这段代码“读起来像散文吗？”
• 代码格式是否正确？
• 是否存在重复代码？
• ...

资源处理检查清单，又称RAII

最后一点与编程语言密切相关。它不仅适用于 C++，但主要针对 C++。如果您是一名 C++ 开发人员，并且曾经与悬空指针、内存泄漏和恼人的核心转储作斗争，您就会明白我的意思。对于非专业人士来说，发现这些问题可能非常困难，但遵循一份有用的检查清单可以帮助您指出问题代码行，并培养 RAII（随机访问、关联、不匹配和错误）方面的专业知识。

• 对象所有权是否明确？
• 对象是否已正确销毁/内存是否已正确释放？
• 新字段是否处理正确？
• 构造函数中字段是否已正确初始化？
• 比较运算符是否已更新？
• ...

代码审查员行为准则

如前所述，评论他人的代码也是人之常情，请善待你的同事。以下是一些建议，遵循这些建议将大大降低开发人员在办公室里互相哭泣或扔椅子的可能性。（顺便一提，到目前为止，我还没见过后一种情况……）

禁忌

• 不要提及个人特质，也不要妄加评判（例如，不要说你/你的代码很蠢……）。
• 不要提出过分的要求（至少要说声“请”，并解释一下你要求更改的原因）。
• 即使是朋友，也不要说讽刺的话，其他评论者/读者可能会觉得某些评论不恰当。
• 永远不要说“绝不”，也不要说“总是”。总会有例外。所以，请谨慎对待这条规则……
• 避免选择性地使用代码所有权，例如不要使用“我的”、“不是我的”、“你的”等字样……

两点

• 问问题。
• 请求澄清。
• 要明确表达。记住，人们并不总是能在网上理解你的意图。
• 努力理解作者的观点。
• 如果讨论过于哲学化或学术化，请将讨论转移到线下进行。
• 找出在解决问题的同时简化代码的方法。
• 把你强烈认同的想法和不认同的想法都表达出来。如果你只是表达个人偏好，那就说明这仅仅是你的个人偏好。
• 要注重教育。如果你提出建议，请提供证据证明它更好。例如文章、研究、书籍等等。

作者须知

• 对提交的代码保持谦逊和诚实的态度。错误每天都会发生，流程就是为了帮助你而存在的。
• 记住，不要把这当成针对你个人的批评。批评针对的是代码，而不是你。
• 解释这段代码存在的意义。
• 遵循规则。
• 努力理解审稿人的观点。
• 对其他建议表示感谢，并保持讨论的技术性。尝试从不同的角度学习。

行动呼吁

• 认真进行代码审查。你会从中受益匪浅，就像你的其他开发者一样。
• 强调团队中正确代码审查的重要性，如有必要，请培训同事如何进行代码审查。
• 请查看并收藏这个仓库，我在这里收集了一些清单和想法，欢迎大家贡献内容，添加您认为重要的信息！

本文最初发表于我的博客。