CodeReview

术语

CR的原则

1. Reviewer作为CR的Owner, 有责任保障CL的质量

2. Reviewer不应该追求完美，而应该追求持续改进

   • 没有完美的代码，只有更好的代码
   • 可以在评论前加上nit:前缀，表示这是一个开发可以暂时忽略的改进点

3. CR的评论应该具有指导意义

   • 明确指出错误或需要优化的地方
   • 对于优化的内容可以给出自己的建议

4. 代码风格应该与现有风格保持一致。如果项目没有统一风格，那就接受开发的风格

   • Go代码风格可以参考Go CR Comments

5. 有分歧难以达成共识时，发起项目组内部的讨论，带上你的Leader

CR的内容

1. CL的总体设计

   • 不同代码段的交互是否有意义
   • 此次变更部分与系统的其他模块能否很好地集成
   • 是否是合适的添加时机

2. 功能验证

   • 功能实现符合PRD TD
   • 功能性的改动对用户是友好的，不应该有breaking change
   • UI的改动是合理且好看的

3. 不应该过度复杂和过度设计

   • 单行代码是否过于复杂、函数（方法）、结构体等是否过于过来复杂，可读性比较差
   • 过度设计：为了让代码过度通用化，超过原有的需求增加一些不需要的新功能

4. 有合适的单元测试

   • 包含对应的单元测试
   • 测试用例是合理且有用的

5. 命名规范，简短易懂，看到名字就能知道内容

6. 合适的注释，注释应该解释why而不是what

7. 遵循统一的代码风格

怎么看CL

1. 用宏观的角度来看待改动，查看CL的描述以及它完成的功能

   • 该改动是否有意义、合理？如果在第一时间认为不应该发生这种改动，请立即说明原因，并给出对应的建议

2. 检查CL的主要部分

   • 通常CL内会有文件包含大量的逻辑改动，而它正是CL的主要部分，比如API、核心路径等等
   • 如果CL的内容过大，导致无法确定哪一部分是主要的，可以先开发询问或者建议他们拆分CL
   • 如果发现主要部分存在设计问题，应该及时评论

3. 用合理的顺序查看CL的其余改动

   • 试着找出一个有逻辑顺来review其余文件，并确保不会错过任何一个

CR的速度

1. 如果你专注于某项工作时，请不要打断自己去做CR

2. 及时回复评论比起快速结束这个CR更加重要

3. 尽量在开发还在工作时间内回复

4. 由于改动过大导致难以review时，可以要求开发将CL拆分成更小的CL

5. 某些情况下为加快review速度，可以给出LGTM或Approval的评论

   • reviewer确信开发人员会适当地处理所有剩余的评论
   • 剩余的评论是微不足道的或它们不需由开发者来处理

6. CR的能力会随着时间进步

   • 如果你遵循这些准则，严格对待CR，你会发现整个CR的流程会越来越快

如何写评论

1. 当开发人员不同意你的建议时，思考一下谁是正确的，并能给出合理的解释，保持礼貌

2. 如果reviewer坚持己见，会让开发者感到沮丧；沮丧很多时候来源于Comment的方式，并非来自于对代码质量的坚持

3. 如果CL引入了新的问题，非紧急情况下，必须在提交之前将其处理掉

4. 如果现在无法解决reviewer评论的问题，加上TODO注释并链接到刚记录的缺陷

5. 发现Bug时，可以将bug链接到对应的comment

评论太严格

提高review的速度会让这些抱怨逐渐消失。这些抱怨可能需要数个月才消失，但最终开发人员会看到严格的review所带来的价值，因为严格的review帮助他们产生的优秀代码。