因为最近在工作上参加制订了团队的一些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实际对我的项目或者集体的倒退都有微小的作用。
集体技术动静
欢送关注公众号进击的大葱一起学习成长