共计 6241 个字符，预计需要花费 16 分钟才能阅读完成。
因为最近在工作上参加制订了团队的一些 Code Review(CR)的标准，所以想在这里给大家分享一下咱们积攒的一些 CR 最佳实际。本篇文章会包含上面这些内容：
- 为什么须要 Code Review
- 什么时候做 Code Review
- Committer 须要留神什么
- Code Reviewer 须要看哪方面的内容
为什么须要 Code Review
对于参加人数大于或者等于两个的我的项目来说，CR 是一项必不可少的流动。在我看来它有上面这些益处：
能够进步 committer 对于本人代码的要求
依照我以往的教训来说，有些人的代码在被 review 和不会被 review 的时候格调是齐全不一样的。如果一个人写的代码不必他人看，永远都是间接推到或者间接合入主分支的话，这个人写代码的时候就不会有什么 顾虑 ，本人想怎么写就怎么写，反正也不会有人看。可是当他的代码须要被伙伴 review 时，他的心态就齐全不一样了，这个时候他会想方法精简代码，去掉一些硬编码的逻辑从而谋求一些最佳实际，而且各种TODO 和FIXME也会写得一清二楚。因为他如果不怎么做的话，作为一个及格的 code reviewer，咱们是必定不会将他的代码合入主分支的。长期如此的话，committer 的代码程度也会逐步失去进步的。
组内常识分享
俗话说的好：你有一个苹果，我有一个苹果，咱们替换一下，一个人还是只有一个苹果；你有一个思维，我有一个思维，咱们替换一下，一人就有了两个思维。这句话同样实用于咱们进行软件开发。作为一个程序员，咱们在面对同一个问题的时候，可能会有不同的解决办法。如果大家没有交换的话，每个人可能永远都只晓得一种解决办法，大家也就没有提高可言。那么程序员之间如何互相学习呢？最简略的方法就是看他人的源代码，而 CR 就是最好的浏览他人源代码的过程。因为当咱们在帮 committer 进行 review 的时候，他提交的代码只专一于解决一个问题，这样代码量就不会很大，咱们能够更加清晰地学习到他是如何解决某一个问题的。如果 committer 恰好用了一个很奇妙的咱们之前不晓得的方法来解决问题的话，咱们就能够学习到新的常识了。
放弃我的项目代码格调的一致性
同一个我的项目外面不同的程序员因为背景不一样 (可能来源于不同的我的项目，来源于不同的公司)，他们解决问题的办法和思路就不一样。作为一个前端程序员我见过在同一个代码仓库外面同时应用了redux-saga 和redux-thunk作为异步中间件的。如果作为 code reviewer 咱们不对 committer 的代码进行束缚的话，我的项目的代码格调就会多极分化，这无疑会 减少我的项目的保护老本以及前面新退出开发者的学习老本。
提前发现代码的问题
一些教训比拟少的开发者在写代码的时候可能思考问题不够全面，导致一些边缘状况 (edge case) 没有思考到，这时候如果 code reviewer 是一个工作教训比拟多的同学的话，就能够帮 committer 提前发现问题，这样就能够防止在产品上线后再浪费时间去定位问题了。值得注意的是，就算 code reviewer 不是一个很资深的开发，他作为代码的局外人往往也能够思考到当事人 (committer) 在写代码时思考不到的状况的。
什么时候进行 Code Review
CR 肯定要产生在各种 CI 自动化查看例如 单元测试 ，lint 查看 等通过之后和代码正式合入主分支之前。
这里波及到的一个问题是如果咱们开发的性能波及的改变很多的话，是等咱们都开发完后再一起给 reviewer 看呢还是拆分成一个个小的 MR(Merge Request)给 reviewer 看呢？我集体偏差于后者。因为如果你提的代码很多的话，code reviewer 须要花很多工夫去浏览你的代码，了解各个模块之间是如何合作的，而且给你提问题后，你可能须要改变很多的中央，这就导致整个 CR 周期变得很长，这些其实都会打击 code reviewer 给你挑毛病的积极性，这种状况下，一些没有急躁的 code reviewer 可能就给你一个简略的 LGTM 而后就将你的代码合入主分支了，这样的 CR 是毫无意义的。因而更好的做法是在开发大性能时候将代码拆分成一个个小的模块，每实现一个小的模块就合入主分支，直到所有的性能都合并入主分支为止。可是如果开发者不想将本人 未实现的功能模块 合入主分支怎么办呢？这个时候能够应用一种叫做 stacked CR 的模式：首先从主分支切出一个 feature/big-feature 的分支，你前面在开发这个 big-feature 的时候，都是从这个 feature/big-feature 分支切出各种性能的小分支，例如 feature/big-feature-sub-feature1，feature/big-feature-sub-feature2 等。当你在小分支上开发完后就能够提 MR 将代码合入 feature/big-feature 分支上。这样因为每个 MR 都很小，所以 code reviewer 就能够认真仔细地给你 挑毛病了 。最初等你把所有的性能都合并到feature/big-feature 上后，就能够提一个 MR 将这个分支的内容合入主分支了。这个时候尽管改变还是很多，不过因为 code reviewer 曾经在之前的阶段看过了你所有的更改，这个时候他要看的货色其实就不多了，对他进行 CR 也不会有很大的妨碍。
Committer 须要留神什么
作为代码的提交者，如果咱们心愿咱们提交的代码能够被 code reviewer 认真对待并且失去有用的反馈的话，须要留神上面这些方面:
限度每个 commit 的改变大小和范畴
咱们 CR 的时候，最烦的其实就是看到其他人提交了一大堆改变，因为这样咱们就须要花很多工夫去了解他的逻辑，这也是很多我的项目很难进行无效 CR 的次要起因，那就是从开始提交代码的时候就曾经错了。作为 committer，咱们在提交代码的时候须要将改变管制在一个正当的范畴，我集体的一个偏好是将 改变的文件数管制在 5 个文件以内 ，将 改变的代码行数管制在 150 行以内，这样的话 reviewer 就不须要破费太多工夫来帮咱们 review 代码，并且也乐于给咱们提供更改意见。除了改变大小的限度，咱们同时也要留神 commit 的范畴，确保每一个 commit 都只做一件事。举个例子: 作为一个前端程序员，你在实现一个导航组件的时候，不要顺带修复一两个 bug，或者更改一些配置信息，你应该将你这个 commit 集中在导航组件的实现下面。换句话来说就是：你要保障你的每一个 commit 只实现某一个性能或者修复某一个 bug，不要将它们"捆绑销售"。
Commit 信息要有意义
很多开发者在 commit 的时候都不会认真写 commit message，特地是在外企要求你用英文写的时候：）。工作多年，我见过一些很懒的开发，他们所有的新增性能都叫feature 或者是 add a new feature，所有修复的 bug 都叫bugfix 或者 fix some bug。这些 commit message 是没有什么意义的，因为它们没有通知 code reviewer 这些代码到底开发了什么新性能或者修复了哪个 bug。要编写一个好的 commit message，首先得给它固定一个格局。当初业界比拟推崇和闻名的 commit message 格局是 conventional commit, conventional commit 的次要作用是将咱们所有提交进行分类，例如feat 代表 feature, fix 代表 bugfix 等等。当咱们的 commit 分好类后，除了便于浏览，还有一个益处就是一些自动化生成 changelog 的工具能够很不便地依据咱们某个版本内的所有 commit 的信息生成这个版本的 changelog。除了所有 commit message 都要遵循某一个固定的格局外，咱们 还须要在 commit message 外面写明确咱们的代码到底实现了什么性能或者修复了什么 bug。如果你在某个 commit 外面实现了一个用户注册的性能，你的 commit 信息能够写成 feat: implement user signup logic 而不是 feat: add new feature，如果你修复了登录按钮不能够点击的 bug，你的 commit 信息能够写成fix: login button can not be clicked 而不是fix: fix some bug。
Code Review 须要看什么货色
那么作为一名合格的 code reviewer，咱们在帮我的项目成员进行 CR 的时候须要看哪些内容呢？依据我集体的教训，我总结了上面这些方面的内容：
正确实现了性能或者修复了 bug
作为 code reviewer，咱们首先要保障的是，committer 提交的 MR 正确地实现了某个性能或者的确修复了某个 bug。在咱们理论开发的过程中，因为信息传递的不统一或者是开发者了解存在偏差，committer 在提交代码的时候可能没有实现完所有的性能或者没有彻底修复 QA 发现的 bug，这个时候咱们就能够在做 CR 的时候指出来避免到 QA 测试的时候甚至是上线的时候再发现问题。不过要留神的是，这不代表 committer 不须要自测，相同 committer 本人在提交代码的时候须要尽最大致力保障本人的代码性能上是没有问题的。
确保编码格调统一
这可能是 CR 最根本也是最重要的一个目标了。作为一个前端程序员尽管咱们可能有一些诸如 eslint 的工具去帮忙咱们确保代码 格局统一 ，例如 statement 前面要不要带分号，缩进是 4 个空格或者两个空格等。可是因为格调不只是格局，有很多编码格调的问题是不能用 eslint 这种自动化工具进行束缚，而是要靠 code reviewer 在 CR 阶段指出来。因而作为 code reviewer， 咱们须要在 CR 的时候确保合入主分支的代码格调根本保持一致，防止不一样的编码格调升高我的项目代码的可读性和可维护性。
防止非必要的简单逻辑设计
因为在同一个我的项目外面每个人的程度和工作教训都不一样，所以面对同样的问题，有的开发者会用十分优雅和简洁的解决方案去实现，而有的开发者受制于本身的能力，会想出一种非常复杂的解决方案。无必要的简单的代码逻辑会升高代码的可读性、可维护性和鲁棒性。作为 code reviewer，当咱们看到适度简单的代码时肯定要指出来，而后尝试和 committer 一起想出一种更加简略的解决方案(参考开源计划等)。
防止硬编码
硬编码的逻辑和值会升高代码的可读性和可维护性，并且会减少 bug 呈现的概率。因而作为 code reviewer，当咱们看到一些硬编码的逻辑或者一些诸如 magic number 等 hard-coded values 时，能够叫 committer 将逻辑设计得尽量通用或者将硬编码的值定义为常量值。
进步代码复用率
当咱们我的项目有很多人参加的时候，开发者在实现某个性能时候可能会反复造一些前人造过的轮子，在前端我的项目外面能够了解成一些 公共组件 ， 公共 hook或者是 帮忙办法 等。反复的代码一旦合入主分支会升高代码的可维护性并且也会容易引发 bug(想一下如果某个需要变了，你要改多个中央就晓得了)。因而作为 code reviewer，咱们在 CR 阶段的一个重要工作就是辨认出代码的反复逻辑 。简略来说，反复逻辑包含三种。第一种是 committer 提交 CR 的代码和代码仓库现有的代码逻辑产生了反复，这个时候揭示开发者复用之前的逻辑就能够了。第二种是开发者提交的代码外面有反复的逻辑，这个时候须要开发者通过 抽取公共函数或者组件 的办法来进步本人代码的复用度和内聚度。第三种状况是开发者提交的代码和现有的逻辑有局部重叠，可是现有逻辑不能间接被复用，这种状况须要 code reviewer 和开发者进行单干，想方法对旧逻辑进行革新从而满足新的需要。举个例子，如果咱们现有代码仓库曾经实现了用来展现谬误音讯的 Notification 组件，committer 在新的需要外面提交了一个全新的用于展现正告的 Notification 组件，这个组件的款式根本和之前的统一只有一些细小的文字色彩的区别，从新写一个的话就意味着当 UI 设计师更改 Notification 的款式时，咱们须要更改两个中央，这是齐全没必要并且低效的。这个时候作为 code reviewer，咱们要让 committer 批改之前 Notification 组件的逻辑让其变得更加通用，而后在新的需要中复用这个组件。当然更改之前组件的逻辑和咱们新的需要须要放在两个不同的 commit 外面，因为咱们要限度每个 commit 的改变大小和范畴。
写上必要的代码正文
好的代码正文是用来通知他人你为什么写这样的代码，而不是通知他人你的代码做了什么 。如果你须要一步步写明确你的代码做了什么的话，首先曾经阐明你的代码不及格了，因为好的代码格调是会让人通俗易懂的。相同当咱们为了克服后端接口的一些 bug，或者绕开框架的一些问题而写的一些奇怪逻辑时，咱们才须要通过代码正文来通知他人你写这些代码的目标是什么。除了这些，你也能够应用一些诸如FIXME 和TODO的关键词来通知他人某段代码还有能够进步的空间或者某个性能还没有齐全实现。这里值得一说的是，尽管很多开发会用 FIXME 和TODO等关键词，可是他们喜爱水灵灵地放这么一个关键词在正文外面而没通知其余开发或者将来的本人什么须要做或者什么须要被进步，这种代码正文同样是没有什么用的。因而作为 code reviewer 咱们岂但须要确保 committer 写了该写的正文，同样也要确保他写的正文是有意义的。
带上必要的自动化测试
CR 还有一个重要的作用就是确保 committer 在提交代码的时候带上必要的自动化测试，例如单元测试和 e2e 测试等。作为一个前端开发，其实很多人是不写单元测试的，其中一个比拟重要的起因是不同于后端接口，前端须要变动太快，刚写完的测试可能下个版本就不能用了，或者是测试用例须要写太多，写测试的工夫比写代码的工夫还要多。然而咱们又不能一点测试都不写，因而测试还是有很多作用的，例如能够帮忙咱们 提前发现问题 ，好的单元测试还能够当做 组件或者函数的文档应用 ，同时测试也能够帮咱们更加 高效地进行代码的重构 。那么什么代码肯定要写测试呢？在我看来， 所有的公共逻辑，包含公共组件，公共 hook 和函数都应该写测试，这是因为公共的货色就意味着应用的人多，如果其中一个人随便改变的话，很容易会造成其他人的逻辑呈现问题，因而咱们须要通过单元测试来确保咱们对于公共逻辑的更改不会引发潜在的 bug。另外一个须要写测试的中央是简单的逻辑。简单的逻辑写测试的目标是让咱们能够尽量模仿不同的输出状况，以进步咱们代码的鲁棒性。因而作为一个 code reviewer 咱们在进行 CR 的时候既要确保 committer 写了必要的自动化测试，同时也满足肯定的测试覆盖率。
总结
在本文中我介绍了一些 CR 最佳实际的内容，其实依照我集体的教训来说，团队规模小需要不紧的时候，依照下面的要求进行 CR 还不是一件比拟难的事。相同当我的项目参加的人数变多了，而且版本迭代速度放慢后，很多团队开始放松了对 CR 的要求，可是实际上往往是这个时候咱们才更加须要严格的 CR 来保障代码格调统一以及防止疾速迭代导致的代码维护性降落等问题 。最初想说得一句话是： 做好 code review 很难很麻烦，不过好的 code review 实际对我的项目或者集体的倒退都有微小的作用。
集体技术动静
欢送关注公众号 进击的大葱 一起学习成长
