CodeReview 是一件很需要耐心和技术功底的活动。
- 不了解业务，很容易沦为代码风格检查；
- 代码量比较大，很容易走马观花，放过业务细节上的纰漏；
- 技术功底不够，很容易放过性能或安全上有漏洞的代码；
- 代码细节不严格追究，似乎达不到 CR 的力度；来回沟通好几趟。
- 需要设定标准和底线。底线达不到，坚决不通过，不因任何问题而妥协。
最难的是，CodeReview 是一项需要全局观的活动。哪怕仅有一行代码改动，也需要在更大的业务语境里去审查其是否恰当；如果有多行改动，恐怕要结合该业务的所有代码去分析。因为，改动的代码有可能单看上去没有问题，但结合已有代码及逻辑，就可能潜藏错误或漏洞。
BUG 无处不在。以人类现有的理解力和审查力，基本不可能避免大型软件里的 BUG 的产生（你无法看到看不到的 BUG）。能够做到的是： 限制 BUG 产生的影响使之微乎其微。
必要前提
- 阅读 CR 相关的文档及说明，熟悉相关业务逻辑及改动意图；
- 最好结合 IDE 来看代码，能够在具体语境里分析改动（需要 IDE 和 gitlab 互动）；
- 代码提交者保证遵循必要的代码规范和风格；有极少量违反可以提醒，大量违反直接打回；
- 代码提交者保证功能的自测通过；
- 代码提交者确保编译通过，否则合进去别人就难受了；
- 代码提交者自己先行 review 一遍，纠正低级错误；
- CodeReview 重点检查必要规范、关键逻辑、质量层面。
常用检查项
规范与风格
- 命名内容： 是否清晰直观表达意图；是否有粗略不贴切的问题；是否与项目约定一致；
- 命名风格： 变量/方法名 - 驼峰式；常量 - 大写+下划线；（新手容易犯）
- 注释： DO 属性是否有加业务含义注释；（建立团队约定）
- 魔数： 使用常量定义；（情不自禁都会犯，要坚持零容忍）
逻辑层面
- 算法：是否有详细说明、引用出处；（推荐）
- 流程：复杂流程是否有相应文档说明；（推荐）
- 异常：是否所有异常都有被处理，堆栈信息是否打印；（必须）
- 关键代码： 要细读，反复推敲；（必须）
- 模板代码： 是否有拼写错误；（要细心）
表达层面
- 嵌套 if-else: 是否能够解开打平；是否可使用策略模式来分离；（推荐）
- 费解的代码：理解起来有点绕弯，不够直观清晰地表达意图；（推荐）
- 建议的方式：可以给出更好的代码实现建议；（推荐）
质量层面
- 健壮性： 主要检查 NPE、越界、复杂字符串解析异常；（NPE 敏感，要确认）
- 可维护： 重复代码，是否可以抽离子函数或模板处理； 过长的参数、方法内部修改入参等行为；（重复代码敏感）
- 统一性： 同一个概念，是否创建了多个不同类来重复表达；
- 单测：是否有必要的单测；单测是否覆盖主要逻辑；
- 性能：是否有循环调用 API 或访问数据库的做法； 是否面临大数据量场景；
- 并发： 是否有多线程访问的可能性；是否有不受控的线程或线程池创建；是否有加同步访问；同步手段是否恰当；
- 安全： 越权访问；敏感信息明文；SQL 注入；
- 日志： 是否有必要的日志信息；是否简洁得当。
其它
- 对于一些比较紧急的改动会留下改进建议，但快速通过，通过后续代码提交解决遗留的问题；
- 可以对Review的评论进行分级，不同级别的结果可以打上不同的Tag，比如[blocker]必须修改， [optional]可选修改，[question]有疑问需要澄清。
- 可以考虑团队统一安装某个代码检查插件，提交之前先解决插件指出的问题，减少 CR 成本；
- 比较大的改动，可以分批提交，比如分成多个分支；
- 评论宜正面积极，给予建议而不是指责；
- 对于新手，更多是给予建议；对于高级工程师，更多给出质量层面的提醒；
- 不要吝啬你的赞扬；好的代码也有必要拿出来让大家一起学习；
方法
- 可以先初步过一遍，检查下是否明显有以上问题；
- 进一步再深入业务核心，去审查业务逻辑是否有问题；
- 将模板代码与关键代码区分开；
- 关键代码要仔细研读；
- 可以在开发的停顿间隔之间进行；
- 把程序运行起来，亲自试一试，或许你会有一些和他们测试时不同的操作，发现一些他们遗漏的问题。