京东云开发者｜代码评审的价值和规范

评审目的

代码评审的目的就是为了保证公司整体代码的健康状况随着不断迭代，始终保持一个较高的水平，所有在评审中使用的工具和流程都应是为此目的而设计的。

评审原则

• 鼓励质疑

• 保持代码风格，遵守开发规范

• 优先设计原则，尊重个人偏好

• 重视每一行代码

• 尽可能采用面对面的形式

评审时机

研发流程应该是严密的、有节奏的，而个体的代码质量会影响整体交付进度，所以请第一时间启动代码评审，最晚不要超过早期测试阶段。

如果是异步评审的机制，评审过程最好不要超过一个工作日，如果评审时间较长，请在开始评审时进行初步反馈。

评审范围

1. 功能

这个 Change List 是否达到了预期目标？

并发、数据权限、性能、竞态条件等一系列边缘异常是否合理规避？

2. 复杂性

新增的复杂是否是值得的？

复杂设计的实现是否是可读的？

抽象定义是否是优雅整洁的？

鼓励通过设计提高可扩展性，但不可“面向未来做设计”，二者之间的界限应该是：是否能够看到明确的演进方向（actual shape）和需求

3. 单元测试

是否有单元测试？

单元测试是否具有良好的可读性？

每一个测试是否有断言？

是否能覆盖尽可能多的逻辑分支？

4. 命名

命名是否符合规范，且具有良好可读性？

命名是否能充分表达一个项是什么、用来做什么？

5. 注释

注释内容是否是必须的？

注释信息是否全面表述对应代码的意义？如果发现注释难以解释这段代码，那么很大概率上这段代码应该简化或者重构。

注释信息应表达代码的用处，而不是解释代码在干什么

6. 代码风格

鼓励对代码风格提出改进建议，但请提及这是一项锦上添花的建议，切不可作为评审通过与否的判定条件。

如果使用评审工具，请在评论前标注Nit:，以标识这是一项 Nitpick（吹毛求疵）的建议。

7. 文档

是否同时建立了或修改了相关文档？

文档格式是否与原项目保持一致？

8. 上下文

修改的内容是否影响原业务逻辑的上下游依赖？

修改的内容是否导致代码质量下降，甚至系统架构腐化？

9. 优秀的代码设计

请不要忽略 change list 中你觉得不错的部分，肯定优秀设计比指出错误更有价值。

评审尺度

不要为了提高评审速度而牺牲代码评审的标准，团队内的代码评审应该是一个持续改进的过程，发现问题、解决问题、避免问题，这种正向循环会为研发流程的每一步都带来收益。

如果因为各种原因确实需要加速评审环节，可以按照重要程度降低一部分评审标准。但请在合适的时间，对这部分代码进行重新评审，项目进度紧张不应成为降低代码质量的理由。

如何解决评审意见冲突

评审是对他人工作进行评判，难以避免意见相左的情况发生，通常研发人员会有非常多的理由拒绝评审建议。

1. 谁是对的

如果研发人员认为评审结果有问题，评审人员请优先思考开发者是不是对的，毕竟他们“离代码更近”。

如果评审人员认为评审结果是正确的，合理、适当、礼貌的讨论，会让真相更清晰。

研发人员的反感情绪通常是因为提出问题的方式，而不是对代码质量的坚持。

2. 稍后再解决

研发人员最常见的拒绝原因，就是进度紧张，希望能够先做妥协，承诺后续修正。

但通常之后不会再去做这件事，这并非完全是责任心的问题，而是因为研发人员通常非常繁忙，修复这件事就容易被遗忘。

所以最好将评审建议尽快修复。

3. 评审过于严格

如果评审尺度严格导致研发人员抱怨，那么礼貌的坚持非常有必要，严格的代码评审有助于产出优秀的代码。

可能过了很长时间后研发人员才能看到这部分代码评审的价值，经过论证后的价值观一致更容易建立彼此的认同感。

总结

代码评审是一项具有长期价值的工作，并且对评审双方都具备价值。不要惧怕提出问题，这更容易提高你对问题的认知，如果最终发现你提出的问题是错误的，这对你也是一项难得的提高。更不要拒绝修改问题，即使这些问题在你看来微不足道，反复的正向行为形成惯性，更容易提高工作质量。

作者：康志兴

参考：

1. https://google.github.io/eng-practices/review