Code Review 代码复审指南
jopen
8年前
一开始这篇指南是为团队而写的，后来发现包含的内容比较有一般性，所以删掉了一些和团队强相关的内容，发到这里。
「Code Review」直译过来便是「代码复审」，也就是让代码作者之外的人来复审代码，提出修改意见，接受代码变更。
益处
- 发现错误：一个人的思考总是会不可避免地出现一些纰漏，而这些纰漏在另一个人眼中也许显而易见。
- 保持代码风格统一：对于整个团队来说，代码风格的统一显得至关重要。风格一致的代码能提供更好的可读性，也能避免犯一些「低级错误」。
- 保证设计的合理性：代码直接反映出你的系统设计，在写一个新的系统时，确保你的设计不会出现大问题。
- 保证提交的变更符合团队的其他要求：例如对测试用例的要求和对文档的要求；
- 互相学习：Code Review 的过程也是思想交流的过程，可以取人之长，补己之短。
怎样进行 Code Review
Review 时机
基本的语法检查和自动化测试通过之后，代码被合并到主分支之前。
Reviewer
Reviewer 可以是一个，也可以更多。在代码变更越大、变更内容牵扯到更多的人和外部系统，或者自信度越低的情况下，你就可能需要更多的 Reviewer。另外在不同的场景下，你也需要不同角色的 Reviewer 参与。
- 新人上手：Mentor 是第一选择；
- 日常工作：同组的其他工程师；
- 向其他系统或模块提交 Patch 或 Bugfix：相应系统或模块的 Maintainer；
- 专业度很高的代码：团队中相应的领域专家。
内容
Reviewer 需要针对变更作以下复审：
- 代码风格：确保变更代码风格符合规范。
- 明显的错误：确保变更代码不出现低级错误，例如访问不存在的属性或访问空指针。
- 不合理的设计：确保变更代码不出现明显的不合理的系统设计，例如单个 Redis 实例存储过多的数据、滥用 OOD 或动态语言特性。
- 明显的性能问题：确保变更不会引发明显的性能问题，例如连续、大量、串行地调用 RPC 接口或者不合理的存储访问模式。
- 测试用例：确保相应的变更有相应的单元测试。
- 注释和文档：确保有必要的注释和接口说明文档。
- 其他团队约定：确保变更的内容符合团队的其他约定，例如服务的接口的访问需要接入监控系统和日志系统等。
如果被 Review 的内容对应一个新的系统或功能，Reviewer 还需要确保：
- 理解这个功能的细节。
- 理解这个系统的工程设计细节；
合并时机
通常来说，需要所有的 Reviewer 都接受变更才能合并代码。
Review 文档
- Python 代码规范
- MySQL 使用规范
- Redis 使用规范
- …
编写 Review 友好的代码
高效地进行 Code Review 不仅仅只是 Reviewer 的工作，代码的作者也起着至关重要的作用。
Commit 粒度
总的来说，Commit 的粒度应该遵循以下原则：
- 尽可能小：一个 Commit 的变更应该尽量少，一个 commit 只做一件事情，如果一件事情可以被拆分成多件事情，那么它们应该对应不同的 commit。例如，不要将「修复一个 Bug」和「重构一个函数」放到同一个 commit 里。
- 自成单元：一个 Commit 应该自成一个单元，例如，不要将一个新增的函数放到多个 commit 里。
Commit Message
Commit Message 应该包含两部分内容：
- Commit 的变更总结：变更内容本身就已经说明了详细变更，所以在 message 里面只需要一个变更总结。
- 为什么要进行这次变更：这是更重要的一部分，因为在大多数情况下，变更内容并不能说明为什么需要做这次变更，知道变更原因才能更好地理解变更内容。只有当变更原因特别显而易见时才能省略此部分内容，例如修复一个不存在的 API 调用。
Commit Message 的推荐格式为：
变更内容总结 变更原因详细说明...
例如：
Refactor foo module with gevent. We met performance issue with foo module recently, foo handles client requests one by one so we just use gevent for concurrency.
坏的例子，如：
Fix a bug. |
这个也好不到哪儿去：
Update config. Update the 'host' config to `b_server` from `a_server`.
Review 单元粒度
一个 Review 单元包含一个或多个 commit。
Review 单元的粒度应该遵循以下原则：
- 尽可能小：一个人连续高度集中精力的时间通常不会太长，所以更小的 Review 单元能够得到 Reviewer 更细致的复审，Code Review 的质量就会更高。
- 能够安全地并入主分支：Review 单元在被 Reviewer 接受之后应该尽快并入主分支，这样才能更好地实现持续集成和持续交付。因此，一个 Review 单元应该要能安全地并入主分支。