• CodeReview最佳实践
• code review的目的
• code review不该做的事情
• 谁需要code review
• 不适合code review的情况
• code review前需要声明的原则
• code review 的分支管理
• 谁来code review及合并代码
• 谁来code review
• 谁来合并

• 我的代码可以提交PR了吗，代码提交PR判断标准
• code review实施方式
• code review产出，回馈，激励
• code review清单（checklist guide 可持续补充）
• 可维护性
• 错误处理
• 安全
• 控制结构
• 可重用性

CodeReview最佳实践

怎么做好code review，codeReview有哪些指导原则，团队如何开展codeReivew，是集中式还是在日常开发中进行，本文试图展开讨论。

这是一篇旧文，但ＣodeReivew思想不过时。

code review的目的

提高代码质量，将code review学习最佳实践的机会大家互相学习，互相监督进步的有效方式

code review不该做的事情

• 讨论设计，设计应该前置，在desgin view阶段解决，不在此阶段进行，在编码前就已经确定，设计部份是在需求明确后，大家一起制定的时候决定
• 情绪化批评指责(你写的真是垃圾，真是笨)
• 保证业务逻辑正确，业务的逻辑边界正确性不应该由code review来保证，也无法保证，应由自己的单元测试和最后提测阶段保证

谁需要code review

• 技术驱动型团队：一般涉及系统底层逻辑较多，功能路径难以被测试覆蓋，而产品质量问题很多时候是致命的，所以这样的团队更多需要开发编码的严谨性和相关代码质量的保证活动。手机管家高权限应用组就属于这一类型。
• 公共服务型团队：一般服务于多个团队，一旦出现质量问题影响范围会比较广，所以除了在测试方面加以把关外，通过CodeReview活动来提升开发质量是非常有必要的。
• 测试缺失型团队：这样的团队由于缺乏测试环节，质量问题带到线上的风险会很高，强烈建议在开发环节做好自检工作。
• 新人密集型团队：新人的代码可读性往往是比较差的，特别需要组织能及时给予纠正，帮助新人养成良好的编码习惯。同时如果团队产出的代码可读性较高时，新人也可以更快上手工作。
• 任何有主观意愿的团队：这样的团队或领导者认同CodeReview的意义，或团队成员对代码质量提升有追求。

不适合code review的情况

• 创新型团队：这种团队的重要任务是要把产品快速推向市场进行价值验证，所以在代码编写上要求足够敏捷，代码暂时的混乱完全可以接受。
• 不认同型团队：即领导和团队骨干都不认同CodeReview意义的团队，这样的团队无论从推动还是坚持上都有很大挑战。
• 疲于应付型团队：这种团队一般没有建立必要的持续提升机制，每天淹没在各种需求沟通实现变更和优化中，自然，代码质量提升活动也很难被列入backlog。

code review前需要声明的原则

• 对事不做对人
• 原则上代码review不对业务逻辑进行review，如果顺手发现逻辑bug也可以提出来，不对设计进行讨论，如果发现了，也可以提出。
• 每条review给一条正面评价(比如点赞)
• 保证commit comment和review comment的可读性
• 全员参加 Code Review，并设定各部分负责人
• 0.5d 或者 单个job完成时提pr，或者 200行提交(任务较大)
• 不能将超过2d的进行累积提交pr，这时需要向团队解释原因
• 每个代码 PR 内容一定要少。Code Review 效果和质量与 PR 代码量成反比，你一下提交这么多代码，我今天还下不下班了？ 我女朋友你帮我陪？每次 PR 代码量小一些，看起来速度快，又不至于失去耐心，这样才能达到 Code Review 的效果，所以要经常进行 Code Review，但是每个 PR 代码量要少。我建议要少于 300 行/PR或者0.5d/PR
• 在写新代码之前，先 Review 掉需要评审的代码。你让我去 Review 一周前的代码？我还得把思维和项目进度切换到一周前？大家肯定不愿意，所以要形成规定，写新代码之前先把旧的 Review 掉，提交 PR 的时候也保证代码量小，这样 Review 起来不需要大块时间，改起来也快。不能因为 Code Review 大幅耽误项目进度，进度是全团队的事，不是某个人的事
• 如果你有更好的方案，尽管提出来。在 Code Review 中经常会发现写的不好的地方，如果你有更好的方法，欢迎提出来！首先能改进这个 PR 的代码，其次能体现你的能力，团队应该定期对这种提出好的解决方案的同事进行奖励。
• 不要在 review 中讨论需求，review 就是 review。不要在 Code Review 里搞别的，有需要就另安排时间进行，要明确 Code Review 是完善代码，不是需求和功能讨论，始终要以代码质量为中心。

code review 的分支管理

• 建议由项目负责人或者功能负责人创建一个feature/xxxx/master的分支，这个分支作为功能基础分支，
• 开发人员可以通过这个分支创建自己的开发分支 feature/xxxx/aaa
• 开发人员在自己的子分支上开发，开发一些代码后符合pr条件后，向feature/xxxx/master 提交PR
• code review 人员收到通知（邮件或者口头）后，进行review代码，这一阶段，所以都可以参与review，合并权限可根据团队规模来指定多人来合并
• 通过git lab上面设置分支的正则匹配来管理功能主分支设置分支保护，防止被push

谁来code review及合并代码

谁来code review

• 团队资深开发人员
• 架构师
• 所有人

谁来合并

• 团队资深开发人员
• 架构师
• 项目owner

我的代码可以提交PR了吗，代码提交PR判断标准

• 在编码代码时，请问自己以下基本问题：
• 我能够轻松理解 代码吗？
• 他人能轻松理解我的代码吗？
• 代码是按照编码标准/指南编写的 吗？
• 相同的代码 重复 两次以上吗？
• 我可以 轻松地单元测试/调试 代码以找到根本原因吗？
• 这个功能或类 太大了吗？ 如果是，函数或类是否有太多责任？单一职责原则？
• 常量是否hard code在代码中，方法命名是否遵守了约定规范

如果上述问题不能很好回答，建议重新组织和设计你的代码

code review实施方式

• 强制&非强制： 按照经验，CodeReview启动前期建议采用强制要求，否则很难有效开展起来。坚持一段时间待习惯养成后再考虑自由度。
• 小片段&大模块：如果想要让问题暴露更充分或降低review的难度，建议采用细粒度方式进行，即小片段提交小片段review。如果更关注全局设计和逻辑思路的学习和找茬，那么可以用模块方式统一review。但很多时候这两种方式是可以结合运作的。
• 线上交流&线下会议： 如果想提高效率，建议采用线上方式进行交流，这里要推荐公司的Code平台，上面支持CodeReview的功能都已经比较齐全。如果更喜欢全员一起找茬的那种快感，那么可以采用线下会议方式开展，但采用开会的方式，一般成本较高，可看团队接受度。
• 事前&事后：这里指的是发布前还是发布后。版本发布后统一进行CodeReview的方式更多是一种代码交流活动， 起不到代码质量把关的作用。反之，如果在版本发布前就对代码进行CodeReview，就可以对质量问题起到很好的把关作用。这里是时间和质量之间的权衡。
• 高频率&低频率：笔者建议的是把代码交流放在每一天，所以频率越高越好。具体根据团队实际情况进行安排即可。
• 架构师，资深开发, owner把关code review，

code review产出，回馈，激励

• 什么写法可能导致性能低下？
• 哪个接口要慎用？
• 哪些设计方式需要规避？
• 什么习惯容易引发内存泄漏？
• 等等。。。

每月会从CodeReview提交次数和发现问题数等纬度进行质量之星选举，礼品包括了书籍，公仔，徽章等等，效果不错，做法供大家参考。 代码规范、检视指南、总结优化和激励机制 是code review能正向循环的保

code review清单（checklist guide 可持续补充）

可维护性

• 在执行代码审查之前，请努力了解代码应该执行的操作。
• 要求开发人员在通过代码字面意思无法知晓逻辑的情况下
• 代码是否符合公认的本语言约定
• 代码是否符合公认的最佳实践
• 代码是否符合公认的注释约定？
• 是否存在硬编码

错误处理

• 代码是否符合公认的异常处理约定。
• 代码是否只是捕获异常并记录它们
• io资源是否没有正确设置超时(比如http请求)

安全

• 资源泄漏（io资源未正确关闭）
• 在循环外面开事务
• 是否存在sql注入

控制结构

• 多层if嵌套，及多层嵌套循环

可重用性

• 重复逻辑有没有提出来，是否存在到处复制代码
• 是否存在大段复杂逻辑，有没有分解到最小粒度
• 是否强依赖，是还通过接口来降低依赖