本文主要参考学习 google 开源的 CR 实践指导。 原文有些地方太冗长，因此做了些精简，但未破坏整体结构，如果您碰巧看到，希望对您也有用；
Code Review工作：分为评审人( Code Reviewer )指导和提交者（CL Author）；
评审人需要关注（看哪些东西）
1. 看哪些内容
• Design 设计是否合理；
• Functionality 符合作者的设计目标，功能是否对用户友好； 1）预先测试过； 2）评审些边缘场景，并发问题等； 3）影响UI的代码，如有必要可以加demo演示； 4）尤其注意并发控制的代码评审，因为难以测试保障；
• Complexity 代码是否易于理解，方便以后新人维护。 1）意味着难以理解，和容易以后引入bugs； 2）典型是“过度工程化”；
• Tests 代码逻辑是否正确，且有设计良好的自动化测试用例； 1）测试代码也是代码，同样要简单设计； 2）有断言
• Naming 变量，类，方法等命名是否合理；
• Comments 注释是否清晰且有用； 1）注释说明不应该是“解释代码做了什么”，而应该说明“这块代码为什么存在”，代码不能自解释，你就要尽量让它变得简单易懂； 2）但又不同于 class，modules，或者functions的“文档”，文档可以说明“做了什么”，“会发生什么行为”；
• Style 代码样式；
• Documentation 是否同步更新了相关的文档；（readme，使用文档等）
• 每一行&上下文； 1）大部代码你需要一行行阅读； 2）有时需要看出来变化部分的整体，获得全局的认识；
除了上面的几个层面，具体的资料推荐：《阿里巴巴Java开发手册》
2.如何写 review 的评论
• 礼貌。 （评论只针对代码不对人；）
• 解释你的理由。
• 给出明确修改方向 和 让开发者决策 保持平衡；（代码作者更接近代码；）
• 鼓励开发增加评论以及简化代码，而不是尝试给你解释复杂性；（如果对一段代码不理解，尽量也不要让开发口头解释或在评论里回复解释）
3. 开发者对 review 问题不认可
• 谁对？ 多沟通，开发拥有更多需求和代码上下文，评审者旁观者清，但目标是提高代码质量
• 怕得罪开发者。 如果结果好的，一般不会，但也要注意表达方式
• 推迟处理问题； review的问题尽量立刻修复处理。 如果紧急情况，可以先通过，但必须单独再补一次CL修复问题。问题在于一般后续补CL的概率不高，所以必须同意通过前，让开发者记个issue，同时最好代码注释里加 ”TODO“
• 对review过于严格的抱怨； 1) 评审同学要加快每次review的速度，不要使得整个过程过于冗长。 2) 长久之后会发现严格的价值和好处。 3) 发现价值后一般当初反弹最大的同学，反而容易成为最忠实的卫道夫）
• 解决冲突； 1）需要达成共识； 2）可以进一步当面沟通； 3）可以引入第三者加入仲裁； 4）最后上升引入更高或更广泛地讨论）
4. Code Review的标准
cr目标： 确保持续地提高代码库的代码健康度（质量）。 有几个点需要平衡：
• 开发者，必须能持续的做出改进。 开发者无法做出改进，或者CR门槛过高，开发难以贡献改进，都无法达持续提高的目标；
• 评审者，必须确保评审通过的CL确实有利于改进代码质量，这个同样很难，因为很多”紧急需求“的容易打破计划； 因此最重要的规范: 评审者必须确保允许的CL 是确定可以既有系统的整体代码质量的，即使这个CL可能不完美；
• 即使一个良好设计的代码，但如果不适合加入既有系统，评审这可以拒绝； 指导： 1）评审过程中可以分享一些知识（语言特性，工具和框架等）； 2）看到最佳实践记得分享； 看到典型缺陷记得记录，让更多人学习到！
5. CL如何评审的指南
• 第一步，总览变化 CL 是否讲得通有意义？描述是否清楚？
• 第二步，检查CL的重点部分 1)识别出重点部分，比如代码变化最大的文件，如果变化量太大，建议开发者把1个CL分为多个CLs； 2)识别重点部分是重要的，一般重点部分设计有问题，其余代码上浪费时间就没太大意义；而且有可能开发者后续CL代码会基于重点部分，继续写很多代码； 另外大的设计变更相比小的变化往往需要更长时间
• 第三步， 完整地按顺序评审其余的代码 如果确定核心变更没问题，就可以按顺序走查所有代码。另外建议可以先阅读测试用例，你可能会知道开发者做某些变更的目的。
6. 评审速度
个人独立的研发速度，没有能够提供团队速度重要。如果代码评审太慢的坏处： 1）团队速度下降； 2）开发者会开始抗拒代码评审（大部分抱怨是可以通过加快评审速度解决）； 3）影响代码质量，可能会抑制提patch，持续好的改进的发生；
• 那评审者应该多快呢？ 没有重要手头工作，最好立刻做掉。否则建议在一个工作日（非自然日）搞定。
• 速度 vs 中断 如果你正在写代码，或需要集中精力的联系工作，不建议中断去做评审，可以等到阶段间歇来做。
• 快速响应 如果你确实没有大块时间来做，还是尽量先给出个阶段反馈，让开发知道你在处理了，或者你会逐步处理；
• 跨时区评审 尽量在开发者还在办公的时间，如果他们回家了，尽量在第二天上班前完成评审；
• 评论 LGTM 有几种情况你应该给出允许（LGTM），即使还有待修复评论： 1）评审者确信开发者已经很好记录了剩下的评论问题； 2）剩下的问题不是很重要，开发者不是非得做。
• 大 CLs 尽量拆成多个小的CLs； 如果很难做到，至少要对整体设计给出评审意见；在不牺牲代码健康的前提下，评审人要确保不block开发者做进一步工作；
• 代码评审是逐步改进的过程 整个reivew过程多经历几次，慢慢开发者和评审者就会形成共识，整体质量和速度都会逐步提高； 长期来看，任何短期的再标准和质量上的妥协都是有害的；
• 紧急情况 线上问题修复，处理紧迫的法律问题安全问题修复，可以跳过整个流程；
代码提交者需要关注
1. 写个好的 CL 描述
CL描述： 做了什么变化，为什么做这个变化；这将是存在版本控制的历史记录中，被很多后人反复查看。 以后开发者可能会通过描述来搜 CL，没有描述对应定位一个CL是困难的。
• 首行 1）做聊什么的简单概要 2）完整句子，祈使句式 3）紧跟着空一行 例如：”Delete the FizzBuzz RPC and replace it with the new system.” 而不是 “Deleting the FizzBuzz RPC and replacing it with the new system.”
• “内容部分”应该提供有用信息的 应该包括： 修复的问题的简要描述，为什么这是个最佳方案，如果有其他捷径应该同样提及。 其他相关信息包括： 背景信息，比如 bug号，性能测试结果，设计文档的链接；
• 糟糕 的 CL 描述 “Fix bug”是个不充分的描述。 什么bug？你做了修复它？其他相似的描述包括：“Fix build”，”add convenience functions”.
• 好的 CL 描述
• 在提交CL前，再检查下对应的描述 在审核期间，CL可能会发生重大变化。 在提交CL之前，有必要检查一下CL描述，以确保该描述仍能反映CL所做的工作。
2. 小 CLs
• 为什么写小 CLs？
1）更快
2）更聚焦和完全
3）更不容易引入bugs
4）如果被驳回，影响小，更少无效工作
5）更容易合并 （合并冲突）
6) 小的变更，更容易设计好
7）对评审者更少的阻塞
8）更容易回滚
• 什么是“小” 总的来说，是一个自包含的变更。 意味着： 1）最小更改仅解决一件事； 2）评审者需要知道的关于该CL的所有信息； 3）CL 并入后这个CL后，这个系统继续运行正常； 4）难以理解其含义，也是不行的 （比如加个api，没有使用说明和看到，很可能是无用的，难以判断）；
• 什么时候大的 CLs 是ok的 1）可以把“整个文件”删除类似变更，视为一行变更对待，不会让费评审者较多时间； 2）自动化构建工具产生的； 尽量拆分：小的可以自包的变更，分配给不同的评审者，（比如一部分代码，一部分配置；或者一部分序列化处理部分，另外一部是使用序列化的逻辑）
• 单独的重构 尽量把重构相关的工作（比如变量名，类结构调整等）单独CL来做，不要混在“features” 或“bug fixs”中。
• 保持相关测试用例关小对应的小 CLs 不要加测试代码和代码分离开。尽量一个CL 做到变更有对应测试的覆盖；
• 不要中断构建过程； 如果你有多个CLs相互依赖，你要找到个方式使得任何 CL 提交后不影响整个系统，否则造成影响或中断团队其他同学的构建。
• 不能再更小的程度 尽量去思考如何把功能解构为一系列小的变更，可以咨询团队成员给出建议。因为无法拆分的大变更，一般很少是正确的！ 如果都无法做到，那么和评审者沟通，审查过程要保持引入错误的警惕，尽量写更详细的测试用例来保证；
3. 如何处理评审者的意见
• 对事不对人 Review目的在于提高代码库和产品质量。 需要注意对于评论意见不要怒气相向，建议离开电脑一会儿，等冷静了再回复。保持建设性和礼貌。
• 修复代码 如果评审者反映你的代码有些地方不好理解，首先要做的是把代码调整清楚，然后代码不能更清晰，尝试加上相关注释。如果代码注释还是不行，那么就在 codereview 工具上评论回复。 为什么这个顺序，因为评审者看不懂，很大可能以后其他人也看不懂，所以不要上来就口头或者在 cr 工具上解释就完事了。
• 自己思考 不要盲目“自我感觉良好”，心理上不能接受评审者的建议。无论你有多笃定，还是要退一步自习考虑下评审建议的价值。 如果你坚持自己对的。评审者常常是提供些建议，他们希望你自己可以思考下哪个最好。也许你拥有更多上下文知识，那么你可以耐心解释给他们知道。
• 解决冲突 在解决冲突时，首先要做的是和评审者达成一致。
著作权声明
首次发布于此，转载请保留以上链接
