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

yedingding · August 08, 2013 · Last by yedingding replied at August 09, 2013 · 8832 hits

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

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

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 等于不能上线,团队对于这个得有共识。

You need to Sign in before reply, if you don't have an account, please Sign up first.