代码审查机制
[TOC]
什么是代码审查
Code Review已经是很多公司的常规实践，初看上去好像是浪费时间，降低工作效率，其实反之，好处大家有目共睹。它能检查代码的正确性，合理性，安全性，发现隐秘的bugs，让系统更可靠的运行。它能保证代码能有两个或以上的人熟悉，促进知识共享。它能让团队成员互为备份，互相支持，不会有SPOF。它能威慑埋雷的任何想法，杜绝邪念。它能互相学习好的代码，提高编程技能。等等。
代码审查应该是集体行为而不是个人行为，它是一个过程，多个开发人员审查或筛选另一位开发者（作者）编写的代码，以确保
- 代码没有任何错误或问题。
- 符合所有质量要求和标准。
- 代码执行了预期的测试。
- 合并后，它将使代码库的运行状况保持更好。
审查标准
-
该代码改善了系统的整体运行状况
每个更改列表（Pull Request）都会改善系统的整体运行状况。想法是，由于进行了如此小的改进，每次合并后，软件或代码库的运行状况都会得到改善。
-
快速的代码审查，响应和反馈
首先，不要延迟推送（合并）更好的代码。不要指望代码是完美的。如果它的状况可以改善系统的整体运行状况，则请推送。
这里的关键是没有'完美'的代码，只有更好的代码。
好的代码是初级工程师可以理解的代码。伟大的代码可以被第一年的计算机专业的新生理解。代码要追究简单
如果不在一项重点任务的中间，那么请在代码完成后立即进行检查。但是一个工作日是响应拉取请求所需的最长时间。预计变更列表将在一天之内获得多轮的部分/完整代码审查。
-
在代码审查期间进行教育和启发
通过尽可能共享知识和经验，在代码审查期间提供指导。
-
在代码审查中要谨慎，尊重，友善和清晰
至关重要的是，在代码审阅期间，要善良，清晰，礼貌和尊重，同时也要对作者非常清楚和乐于助人。查看代码时，请确保对代码而不是开发人员做出评论。
-
每个 Review 至少给一条正面评价
Code Review 本意是改善代码质量，增强团队成员之间的沟通，但是我一提交代码就有人说我写的垃圾，这很打击自信心啊，也不利于团队成员和平相处。代码有问题，指出问题是必须的，要实事求是，但是有的时候也需要给队友一点鼓励，例如简单的 或者“赞一个”我都很开心了。
-
用工具进行基础问题的自动化检查。
用 Tab 还是空格，用两个空格还是四个空格，函数后面怎么换行等基础问题检查，可以使用 eslint 和Rubocop 等类似的工具进行，团队成员应该把更多精力放在代码规范，代码性能优化等地方。
-
全员参加 Code Review，并设定各部分负责人。
扩大 Code Review 参与面，参与不是说一定去审核别人的代码，可以是代码被审核，也可以是看别人审核意见，这都是学习的过程。并且每部分设定负责人，该负责人对这部分代码质量负责，负责人需要是资深工程师。全员参与 Code Review 可以让团队成员更快的成长，新人在看大佬 Review 代码的过程就能学到很多。
-
每个代码 PR 内容一定要少
Code Review 效果和质量与 PR 代码量成反比
-
在写新代码之前，先 Review 掉需要评审的代码
你让我去 Review 一周前的代码？我还得把思维和项目进度切换到一周前？大家肯定不愿意，所以要形成规定，写新代码之前先 把旧的 Review 掉，提交 PR 的时候也保证代码量小，这样 Review 起来不需要大块时间，改起来也快。不能因为 Code Review 大幅耽误项目进度，进度是全团队的事，不是某个人的事。
-
如果你有更好的方案，尽管提出来
在 Code Review 中经常会发现写的不好的地方，如果你有更好的方法，欢迎提出来！首先能改进这个 PR 的代码，其次能体现你的能力，团队应该定期对这种提出好的解决方案的同事进行奖励。
例子
在下面的例子中，代码块中的建议性审查注释以 //R: ... 的形式标出：
-
不一致的命名
1class MyClass { 2 private int countTotalPageVisits; //R: 变量命名要统一 3 private int uniqueUsersCount; 4} 5 -
不一致的方法签名
1interface MyInterface { 2 /** 当 s 无法被提取时，返回 {@link Optional#empty} */ 3 public Optional<String> extractString(String s); 4 /** 当 {@code s} 不能被重写时，返回 null */ 5 //R: 应该统一返回值：都用 Optional<> 6 public String rewriteString(String s); 7} -
Bugs
1//R: 这里执行了 numIterations+1 迭代, 是故意的吗？ 2// 如果是的话，考虑改变 numIterations 的含义吧？ 3for (int i = 0; i <= numIterations; ++i) { 4 ... 5} -
架构上的关注点架构上的关注点
1otherService.call(); //R: 我觉着我们应该避免对 OtherService 的依赖。我们当面聊一下如何？
审查时机
每次代码提交，向主仓库提交Merge Request 进行代码的 Review
Code Review 工具
- **Crucible：**Atlassian 内部代码审查工具；
- getcodeflow：微软代码评审工具
- **Gerrit：**Google 开源的 git 代码审查工具；
- **GitHub：**程序员应该很熟悉了，上面的 "Pull Request" 在代码审查这里很好用；
- **LGTM：**可用于 GitHub 和 Bitbucket 的 PR 代码安全漏洞和代码质量审查辅助工具；
- **Phabricator：**Facebook 开源的 git/mercurial/svn 代码审查工具；
- **PullRequest：**GitHub pull requests 代码审查辅助工具；
- **Pull Reminders：**GitHub 上有 PR 需要你审核，该插件自动通过 Slack 提醒你；
- **Reviewable：**基于 GitHub pull requests 的代码审查辅助工具；
- **Sider：**GitHub 自动代码审查辅助工具；
- **Upsource：**JetBrain 内部部署的 git/mercurial/perforce/svn 代码审查工具。
参考
https://juejin.cn/post/6869024508796207111#heading-13
https://juejin.cn/post/6844903733797584909#heading-7
https://blog.csdn.net/weixin_33975951/article/details/90334884