分享 让代码审查成为你的团队习惯

yedingding · 2013年08月08日 · 最后由 yedingding 回复于 2013年08月09日 · 7622 次阅读

但凡我接触过的靠谱的团队,无一不是在团队中推行严格的代码审查制度。这个就像是一种习惯,直接融入在团队血液之中。Pragmatic.ly 团队多年的代码审查实践分享

http://yedingding.com/2013/08/08/dig-into-code-review-process.html

共收到 20 条回复

code review有好的工具配合可以更有效率和更容易实践,github或gitlab的PR模式都是非常适合的,特别是他们的UI和dff的查看非常便利。

楼主的博客写的很好 受益匪浅~~

配合Github flow更好用

#3楼 @Yujing_Z 下次我分享我们的 Git Flow,XD

我们使用gerrit 作为专门的code review工具。和楼主一样认可代码审核的效果。在实施过程中也发现一些阻力:

  1. 不写代码的项目经理或产品经理在review例会上,为了项目进度往往要求去掉这个Code Review,因为它不产出价值。有问题应该让QA来把关。在时间紧张,任务重的情况下直接忽略测试,把问题推给QA。
  2. 在Code reivew的过程中,发现并不是所有的开发者都是喜欢Code Review的。他们更倾向于新任务的第一次交付。有问题等QA测出来再说。
  3. 当团队只有1个到2个人这种规模,Code Review变成了A看B,B看A。很累。不如转向多写功能测试。每一个Bug写一个测试覆盖它。

经常把code review当成交流会去了... 😓

#5楼 @xds2000 看完感觉怪怪的。

  1. PM这么搞,说明不是工程师主导的团队,不喜欢这样的氛围,code review他们觉得不产生价值,这种见识的PM你伤的起?为什么会忽略测试,不都是一边写代码一边都测着的么,QA妹子好无辜的
  2. 很多小团队都没QA的,而且就算有QA我感觉也不应该这样甩手给QA,起码自己稍微测一下
  3. 我觉得还好,A看B,B看A,这不就是异步的pair吗。多写功能测试是对的,但是一般一边开发一边就用cucumber写了,不是问题。每一个bug写一个测试覆盖是一个很好的实践

@Yujing_Z

  1. Code Review的价值楼主已经阐述,我的体会是实践反馈。我的意思是说并不是所有人会认可Code Review。PM并不关心Code Review,也无需为它负责。Code Review应该由Project Tech Leader来坚持和推广。
  2. 关于小团队有没有QA,都应该测试的话题已经跑题了,我们这里讲的是Code Review
  3. A和B的Code Review,不是Pair programming,一边些代码,一边些测试。所以我怀疑你(@Yujing_Z )有没坚持过Code Review。两个人的Code Review也会遇到问题,A今天效率不错,B效率差一点。A Review完B的代码,自己的代码 B没心情Review的情况。但项目要前进不能等人的。

#8楼 @xds2000 A和B互相review当然不是真正的pair,但是思路是一样的,通过两个人互相审查提高代码质量,我说他是类似异步的pair有错么

我确实没有一直坚持code review,一周7天只有5天有可能会code review。但我没听说过心情不好就不review了的

@Yujing_Z A和B做Code Review,会遇到问题:A今天效率不错,B效率差一点。A Review完B的代码,自己的代码 B没心情Review的情况。但项目要前进不能等人的。

你一周7天有5天可能会code review,是做了Review还是没做。你如果一直坚持review,就没有遇到问题吗?你是如何解决的。我觉得这才是讨论的价值。

非常同意@xds2000 的说法。code review 本身没什么难的,但要让团队认可code review这回事就很难了。这里说的团队不光指技术人员,还有老板,项目经理等。也许技术人员可以通过code review得到提升,但是对于很多不是技术出生得老板,经理而言能看到的直接价值几乎没有。推动code review认知度是个漫长的过程,得让大佬们尝到甜头。

#10楼 @xds2000

算了,稍微详细的回复一下,当然,这只是我和我们团队用的,未必是最好的。唯一我能说的就是从没影响过项目进度

Code Review有很多很多的实现方法,我们用的是基于pull request的。每一次的pull request就代表着需要一次Code review。并没有规定说今天一定要结束工作前A和B都review掉对方的代码。一般来说不是特别复杂棘手的问题,通过在github的留言,5个回合之内就能决定是reject还是merge了。如果非常的麻烦,不紧急的话,在standing up meeting的时候讨论掉,或者如果有多个人的项目,你就找一个人能顶的,抽个时间和你无论是近战还是远程pair消灭掉,然后再pq。所以说我说5天有可能cr,我真的没有弗扯。。

基本上来说Code Review就是开发的一部分,所以在我这,pm从来就没有cr占用时间这种说法

这样异步的cr,没有人来鸟你怎么办?这我感觉是团队氛围吧,一般我发一个pr,总有至少2个人来看,然后给我留意见,我改完之后再帮我merge了

至于任务的优先级,或者说哪些应该先review,急着merge的,一般我们都分成Blocker, critical, major, minor, trivial这几级吧。一个PR都会对应一个story,无论是bug还是feature。我们用的pivotal/jira 来track story/issue,同时都会有对应的优先级。PM基本上就成天画大饼写故事,然后我们挑着做。

开发过程中我们也使用Jenkins来CI,保证项目环境啊测试神马的不会出错。所以在前面的5级只上,还有Jenkins build fail 这一级。基本上无论什么情况,假如Jenkins build fail了,都要立刻修复。

扯远了。我觉得code review是我开发的一部分,根本不可能分割。所以我没有那些问题。

#8楼 @xds2000 如果没心情写代码呢?

@doitian 是的,我怀疑有时工程师确实没心情写代码,不在状态。休息是最好的方式。

@Yujing_Z 谢谢分享。你忽略了你的团队成员 > 2。当你的团队特别小(2,3个人)的时候,Code Review对工程师的自律是很高的。唯有坚持才能得到好的收益。

#14楼 @xds2000 我团队成员=2的时候也是这样的,就是俩人互搞,养成习惯就好。不过这么小的团队大部分是私人项目了,cr的间隔会久一点

我好像看出来了 @Yujing_Z 你们上面的压力并不大。而@xds2000 会承受着一定项目进度压力。

从来没有code review的团队如果想推广,可以把团队分为两组,一组进行code review, 一组不review。 从bug数量,代码的可维护性,团队成员之间的了解程度,或其他方面来进行比较, 这样慢慢分组推广比较好,到底有什么好处,有参照有对比。

@blackanger 你在理想化的推广。不如强制性地推广来的快。很多时候好的习惯并不是所有人能理解和赞成。

#18楼 @xds2000 我这属于半强制实验性推广,给大家一个目标, 大家都可以比较,到底有没有好处。完全强制性的不行, 会应付差事,起不到效果。

#16楼 @metal 有项目进度压力,人为制造虫子,然后告诉老板都是没有 code review 的后面,233。 我非常同意@Yujing_Z 的说法,Code Review 是我开发的一部分,跟测试一样,是能不能上线这个修改的重要依据。我不要求这个功能开发有测试覆盖,但是 code review 必须做。也不是有了提交我就立刻做 review,这个可以看开发人员的时间安排,但是没 review 等于不能上线,团队对于这个得有共识。

需要 登录 后方可回复, 如果你还没有账号请点击这里 注册