Code Review实践总结

导语：Code Review很重要，很多⼈都这么说，但是许多团队不做照样能做出不凡的业绩。那么到底应不应该做呢？⼜该如何去做呢？

1、为什么要做CR?

2、为什么不做CR?

3、⼀个好的CR，Developer需要做什么？

4、⼀个好的CR，Reviewer需要怎么做？

5、实践中团队如何推进CR？

我司正在推进研发效率提升的专项项⽬，如果说只能做两件事，我想其中之⼀必然是CR。CR好处太多了，除了能有效帮助找到bug，提⾼代码质量，提升研发效率外（限于篇幅不⼀⼀展开了），还在下⾯两个⽅⾯有突出帮助。

1、增强团队战⽃⼒。团队通过相互review代码， 促进了协作沟通、技术分享，和业务理解，打破代码孤岛、业务孤岛，相互学习，保证了同⼀个业务同⼀个服务甚⾄同⼀个模块，有更多的⼈了解、参与、熟悉其中代码的流程、设计和框架。

2、改善团队⽂化。在CR中，每⼀份代码提交都是公开的，每⼀个代码评注也是公开的，“Show me the code”，每⼀次CR都是⼀次协作，只有这种过程才能孕育出透明、互助的⼯程师⽂化。提交CR的⼈，希望⾃⼰写的代码⾜够好，不要被捉出太多的问题。⽽做CR的Reviewer，也希望能提出有建设性的意见，展⽰能⼒。这样就能建⽴健康的技术氛围，培养⼤家的Geek精神，保持对优秀coding的追求！

回答导语的问题，CR不能保证团队的成功，但是可以提⾼效率，保证团队更快地⾛向成功或者失败：）

1、反对做CR的同学往往会说“CR过程长，浪费时间，Reviewer还有很多事要做”，或者说“现在不做CR，也没见产品出什么⼤BUG”。事实上刚开始做CR的时候确实会增加⼤家的⼯作负担，但是当CR逐渐熟练的时候，也就是巨⼤红利到来的时候。⽐如团队个⼈能⼒迅速增强，协作氛围浓厚，Bug数减少，需求开发加快，团队补位能⼒增强等等。

2、有时做CR，部分同学会有⼼理顾虑，“⾃⼰写的的代码，就像本⼈的⽇记，是隐私，内⼼是拒绝给⼈看的。” “努⼒写的代码，Reviewer提了⼀⼤堆建议和问题，感觉被diss了，很不爽。” Code是⼯程师的产品，通过CR提⾼⾃⾝的代码能⼒，技术能⼒才是⼯程师应该关注的。当CR是⼀种⽂化，⼀种氛围时，我相信上述的⼼理问题⾃然就不存在了。

1、代码提交时的描述要清楚地说明这次修改的⽬的和⽅法，要有效地帮助Reviewer了解代码的背景和解决的问题。

2、尽量每个CR都包含较少的代码，修改增加的代码⾏数必须⼩于500⾏，⼀般不要超过300⾏。其实500、300是我瞎写的，⾏数没个准，逻辑简单，⾏数可以多点，逻辑复杂，⾏数可以少点，原则是不要让Reviewer花太多的时间在⼀个CR上。这样Reviewer也不会因为代码⾏太多，有压⼒，不能尽快的review完成。

3、⼀个CR尽量只解决⼀个需求问题，不要把多个需求或问题⽤⼀个CR解决，这样其实会⼤幅增加Reviewer的⼯作量。⽐如解决⼀个⽤户登录的需求，最后把微信、QQ登录、⼿机绑定等流程分别实现，分别提交CR，不然众多逻辑的代码交织在⼀起，Reviewer需要花较⼤成本阅读、理解。

4、提交CR前⾃⼰要再检查⼀遍代码及描述，⾃我review⼀次，避免低级错误，不要提交⽆法通过编译的代码。因为这样很不认真，也显得不尊重Reviewer。

5、如果提交的代码涉及到重⼤的修改或者设计问题，最好提前和Reviewer或者团队沟通，避免因为设计问题⽽做了很多⽆⽤功。常常见到因为已经写了⼤量代码⽽不愿意采纳更加合理的设计，要避免让⾃⼰处于这种困境：）

6、要虚⼼开放，能改的尽量改，哪怕⾃认⽆关紧要的修改也最好尊重Reviewer的意见。

7、遵守代码规范，不要随意破坏规矩，除⾮有⾮常特殊的理由。

1、⼀般review两类问题，⼀类是代码规范问题，⼀类是业务逻辑问题。在G⼚，代码风格的 Reviewer是需要认证的，不是每个⼯程师都有资格的。每⼀门语⾔都有各⾃的认证，认证的⼯程师代表已经掌握了此类语⾔的代码规范，有资格进⾏代码规范的review。有时如果 CR的Reviewer没有代码风格审核资格，那么还需要另外⼀个有资格的Reviewer来做代码风格的review。

2、任何时候先提重要的问题，问题很多时，那么先提最重要的忽略次要的。等重要的问题都解决了再提次要的。另外⼤多数⼈是⽆法接受对⽅⼀次给提出好⼏⼗个问题的，这会让⼈觉得⾃⼰的代码太糟糕，不利于后续双⽅理智探讨。

3、那么什么是重要的问题呢？业务边界、逻辑流程、设计接⼝、代码架构、性能这些都是。逻辑死⾓、错误处理、可读性问题、测试⽤例、代码风格、注释、命名等问题，相对次要。另外除⾮问题较少，否则不要试图把所有问题都挑出来，因为许多问题在做了前⾯的修正就不存在了。⽐如如果接⼝设计有问题，那么就不必要指出接⼝命名的问题了。

4、⾄于什么是好的设计，好的架构，⼀般有⼀些共同的准则（⾜够再写⼀篇了），但实际上并没有什么强制硬性的规定，仁者见仁智者见智，⼤家达成⼀致就好。如果道理说不通， 达不成⼀致，⼀般默认尊重code owner或者Developer的意见。

5、Reviewer提的意见越具体越好，不要吝啬⽂字，不要让Developer费好⼤劲猜你究竟是什么意思。当然对⼯程师来说最好的语⾔就是代码了，Reviewer如果能⽤代码说清楚，就直接上代码，哪怕是伪代码；代码不需要细节，把问题说清楚就⾏了。

6、Reviewer提的意见需要对事不对⼈，绝对要避免对个⼈的评价，⽐如“你这种设计效率太低了，怎么能.......” vs “这个设计效率可能不⾼，因为......”。提出问题时要说出你的理由，有时虽然说者可能显得很客⽓，但听者往往⼀脸懵逼。⽐如Reviewer ”请把这⾥改成这样......”，如果理由不是那么明显，Developer很可能是满脸问号。

7、如果代码符合代码规范，逻辑也正确，那么Reviewer应避免根据⾃⼰的编程习惯来评判别⼈的代码，“难得糊涂”。

8、不要为了提意见⽽提意见，找不出问题没有关系，Developer认真写的代码不容易找出问题，这很正常。

9、Reviewer应该尽量快完成CR，不需要打断当前的⼯作优先完成，但也不要拖太长时间， Developer等着呢。

1、制定或者确定代码规范，公司或业界有不少可参考的。

2、各个业务模块找出各⾃的代码owner，负责本模块的代码审核，拥有对代码审核的最终决定权。如果⼀个模块只有⼀个⼈在维护开发，那么也要给它配⼀个资深Reviewer，实在没有Leader上吧。

3、建⽴CR制度，每个模块的代码修改提交，如果没有经过owner的review，⾮特殊情况（如修复bug紧急提交）不能提交⼊库。

4、CR做好并不是⼀件容易的事情，不是⼀开始就能轻松做好的。技术⾻⼲和Leader要⾸先站出来，积极参与，作为Reviewer认真负责地做好每⼀次CR，逐渐熟悉，逐渐磨炼，为其它同学树⽴榜样。

5、建⽴CR统计看板，对CR做得好的同学给予及时的肯定和绩效上的正向激励。

1、如何界定好的代码和坏的代码（设计模式、复杂度、命名⽅式、注释等等）?

R: 先按规范来；规范不涉及的讲出理由；理由⽆法说服的尊重业务和code的owner。

2、codeReview 之后代码的整体改变，⽐如同⼀⽅法，效率提升了多少，可阅读性，bug减少数。感觉不是有⼀个很好的统计和衡量标准来⽐对。

R：CR的好处⽂中都提了，但是确实有些指标是不好量化的，可能也不需要量化。⽐如团队战⽃⼒增强了，只能说“谁⽤谁知道！”

3、具体的落地实施环节..⽐如提前⼀周或者多久发起codeReview，然后整个团体各⾃提出意见并汇总，展开codeReview，各⾃优化代码之后再进⾏验收。

R：⼀般1-2天就及时回了，不需要提前⼀周，如果⼀个CR⼀周后才能得到反馈，那么肯定是哪出了问题，⽐如1.CR太⼤了，需要分解；2.Reviewer太忙了，那找个不太忙的；3.Reviewer 忘了，你要催催他。

另外，建议发起review的同学，在发起review、提交代码时，comments⼀定要写得简洁、清楚。确实遇到过很多发过来的 review，标题就⼀个单词：“update”，作为 Reviewer 真想看都不看，直接打回，节省 Reviewer 时间，就是节省⾃⼰的时间啊......