Code Review

Code Review

标准

代码审核的目标，是为了让整个项目的代码库，随着时间推移，质量有所进步，而不是发生了退化；

为了达到这个目标，需要做一些取舍：

• 小步迭代胜于追求完美主义；
• 确保代码质量不退化，是审核人员的责任；
• 审核人员对所审核的代码拥有责任和相同的所有权；

原则：只要所提交的代码改进了代码库的质量，即使该代码不完美，也应该审核通过；

质量改进维度：

* 更容易维护；
* 更容易阅读；
* 更容易理解；

经验交流

代码走查同时是一种交流经验的机会，但不要强制遵守某种经验。可以在注释中某些更好的做法，但同时备注“仅供参考”；

原则

• 事实和数据优于看法和偏好；
• 如果有代码风格规定的话，就遵守规定；没有规定的部分，则按作者的偏好；
• 软件设计不属于个人偏好的范畴，而应该遵守基本的原则，除非作者能够有充足的理由，能够证明其合理性；

处理冲突

• 当出现冲突时，双方应根据既有原则达成一致意见；
• 如果有困难，则向上汇报，由其他人开会讨论协助判断；

走查什么

设计

• 是否设计良好？
• 模块之间是否高内聚，低耦合？

功能

• 功能是否能如预期那样工作？

复杂性

是否过于复杂了，复杂的标准为“无法快速看懂代码要做什么”；主要包括以下几个层面：

• 行、函数、类；
• 是否存在过度工程？

测试

• 是否包含单元测试、集成测试、端到端测试？
• 测试本身是否正确？

命名

• 命名是否合适，让人能够直接从名称看出意图或内容？

注释

• 注释是否用来说明原因，而不是用来描述动作？
• 是否为正则和复杂算法写了注释？
• 注释是否和文档有所区分，文档用来详细描述代码意图、使用方法、最终效果等；

风格

• 尽量使用既定的风格指南，除非有特殊原因；

文档

• 如果代码涉及外部使用方式的变化，则检查是否更新了文档（增加或者删除）；

每一行

• 走查每一行代码，而不是假设某些代码能够正确运行，而忽略对它们的走查；
• 如果觉得某些代码过于复杂，自己没有把握确保它们的正确性，则应请求其他更有经验和能力的成员的帮助；
• 如果代码过于难懂，就要求作者解释它们；

宏观视角

• 尝试站在更宏观的角度来思考代码变更可能带来的影响，而不是仅仅看局部出现变化的代码；
• 思考新的代码，是否让整个项目的代码质量退化了？如果是的话，应即时修正它；因为大退化总是由无数小退化累积起来的；

不吝赞美

• 当发现某些代码写得很好时，不要吝啬自己的赞美，在注释中把自己的赞美表达出来；

走查步骤

• 先宏观的了解新提交的代码的目标（想要做些什么），并思考目标是否有意义，如果没有意义，就诚恳的提出，并给出新目标（如果这种情况经常发生，说明团队缺乏沟通，导致成员的工作目标不一致，应该改进）；
• 查看提交中发生最大变化的位置；如果发现设计问题，应该第一时间给作者指出，以免对方在错误的基础上继续走太远；
• 弄清楚目标后，按顺序浏览实现目标的文件；如果可以的话，看主要代码前，先看一下测试，也会对目标的了解提供帮助；

走查速度

• 走查的速度应该越快越好，因为反馈的越快，它带来的负面效果越小，正面效果越大；
• 走查的时间不应越过一天；一般来说，今天提交的代码，第二天应该给出反馈；
• 如果自己当前正在写代码，则不应停下来去做走查，而应该是当自己手头的任务完成后，再走查；
• 如果自己无法在1天内给出回复，则应该救助其他同事帮忙走查，而不是拖延；
• 如果提交走查的代码量很大，则应要求作者将它们进行拆分成多个提交；

如何写走查意见

• 保持谦逊的态度；
• 说明理由；
• 鼓励作者简化代码，或者添加注释；