建立更好的代码审查制度-阿里云开发者社区

开发者社区> 开发与运维> 正文
登录阅读全文

建立更好的代码审查制度

简介: 本文讲的是建立更好的代码审查制度,人与科技之间的交互部分总是那么忽明忽暗,难以捉摸。对于开发科技产品的人来说,更是如此。作为一个资深码农,我在代码审查的时候对于这一点的感触特别明显。
本文讲的是建立更好的代码审查制度,

建立更好的代码审查制度

来自 Rails 2017 开发者大会中的一段演讲

人与科技之间的交互部分总是那么忽明忽暗,难以捉摸。对于开发科技产品的人来说,更是如此。作为一个资深码农,我在代码审查的时候对于这一点的感触特别明显。

大多数开发者们习惯于把他们的代码看成一种艺术品,就好比画家看待自己的画一样,我们的代码总是和我们密切相关。一直以来,我们都被教导说要做一个利他的码农,在代码合并到主分支之前,我们不仅要审查自己的代码,也要审查同事的代码。其实我们都知道这样的审查是对大家都有利的,是一件我们都应该做的事情,而且很多人恰好已经在做这些强烈推荐的事了。

但是谁还记得上一次我们衡量这些方法论是什么时候?我们真的能保证我们的代码审查制度是有效的吗?我们能够保证我们的代码审查制度不忘初心吗?

如果答案是不,那我们如何才能解决这个问题呢?

© geek & poke, http://geek-and-poke.com

不要和代码审查过不去

在我们能真正完全理解代码审查的实际意义和好处之后,我们就能知道为什么会有代码审查这个传统了。网上有大量的有关代码审查最佳实践的研究,不过我建议可以从 Steve McConnell 在代码大全 中的相关研究入手(该书于 1993年出版)。

在他的书中,对于代码审查制度应该起到的作用,他写下了下面这些话:

管理软件工程过程中的一部分就是抓住问题“最低价值”的阶段,即在已投入投资最少且花费最少去解决问题的时机。为了达到这样的一个预期,我们可以使用“质量门”这样一个理念,也就是周期性地测试或者审查来决定是否应该进行到开发的下一阶段。

McConnell 的代码审查研究中最重要的理念就是"代码构建中的集体所有权"。所有的代码都属于团队而不是某一人,并且团队中的所有成员都可以对其进行访问和修改。

代码审查制度的初衷是帮助我们在软件开发中采用集体所有权的思想。换句话说,通过参与控制产品质量的方式,我们每个人都会成为开发过程中的股东。

McConnell 在他的书中提出了一些不同类型的代码审查流程,这些流程可以被任何一个团队采用在日常的工作流当中。强烈推荐 McConnell 的代码大全,真的非常赞。但是这边的话我们就简短的说3种来帮助理解。

1. 详查(正式检查)

详查是一个比较长的审查流程,耗时基本都在1小时左右。整个流程会包括一个公司的把关人(扛把子),代码作者(程序猿),和一个审查员(产品狗)。

当这个审查制度被有效使用时,它通常能发现这个程序 60% 的问题(不管是程序缺陷还是错误)。根据McConnell的研究,相比于不怎么流程化的代码审查,这个制度平均每 1000 行代码能减少 20% 到 30% 的错误。

2. 走查

走查通常持续 30 到 60 分钟,对于高级程序员来说,通常是一个向新手们传授经验的机会,与此同时,对于新手们来说这也是一个机会,可以阐述新的方法论,挑战那些陈腐的、很可能已经过时的假设。

走查有时候还是挺有用的,但是一般而言,走查远没有更加正式的代码复查有效。通常走查可以找到程序中 20% 到 40% 的错误。

3. 小型代码审查

就像这个名字一样,这种审查非常短。但是这都是很有深度的审查,还蛮难的。有时候可能就是更改了1行代码,但是会引起大量的问题。

McConnell的研究发现了下面这些有关于小型代码审查的事实:

一个专门针对单行修改代码审查的机构发现在进行代码审查后代码报错率从 55% 下降到了 2%。一个在 80 年代晚期的通讯机构代码正确率在审查后从 80% 上升到了 99.6%。

McConnell 的一组组数据似乎在告诉我们,每一个开发团队都应该结合这三种代码审查方式。

然而,McConnell 的书是在 1993 年写的。时至今日,我们的工作流程早就已经更新换代了,同行审查也随之更新。但是我们现在对于代码审查实行的方法足够合理完整吗?我们应该如何把理论上的东西运用到实际中去?

为了解答这些问题,我做了大多数码农会做的事:Google 一下!

Markdown

嗯,不错。上图是我在推特上发起的一个调查。

程序猿们如何看待代码审查

在我对这个调查进一步阐述之前,我想先说点什么:我不是一个数据科学家(我希望我是,但是我也许在处理这篇文章反馈的时候能更加得心应手,或许我用 R 语言画图还过得去)。还有一点,就是说我的数据集其实是非常有限的。首先这个数据是我自己在推特上选过来的,另外数据是来自一个基于 branch/pull request 的团队的。

好,那么重点来了:程序猿们是到底如何看待代码审查的?

量化的数据

让我们先来看看这组已经量化的数据。

首先,这个问题的答案很大程度上取决于你问了哪些程序猿。在我写这个报告的时候,我已经收到超过 500 条回复了。

下面是一些根据工作时用的语言分类的结果,包括了 JS、JAVA 还有 Ruby。

根据我一个个问完之后,大家都认为 代码审查对于团队是有好处的

总的来说,从 1 分(非常不认可)到 10 分(非常认可),Swift开发团队给代码审查认可度打了 9.5 分的高分。Ruby 开发团队第二,打出了 9.2 的高分。

当70%的受调查者告诉我他们的 pull request 在被 merge 之前会由团队里的其他人来检查时,有10%的受访者(大概50个)告诉我的说他们的 pull request 只有在自己要求进行代码审查的时候才会进行代码审查。

这张图片通过不同语言阐明了代码审查的深度。总的来说,每个语言之间没有差很多。换句话来说,决定代码审查的的深度和频率和你用什么语言没有关系,重点在于你在什么样的一个团队。

最后,对于那些当有要求进行代码审查才会进行审查的团队来说,大多数团队通常只需要 1 个人在代码合并到主分支之前审查这个代码。

定性描述的数据

那么对于那些不可量化的东西来说,我们能知道什么呢?在多项选择题之外,这个调查同时也能够让受访者填写他们自己的答案。这一部分也是这个调查最重要的一部分,最能说明问题的一部分。

这些回答具体聚焦在如下几个重点。 �

总的来说,对于代码审查制度有很大关系的有两个因素:执行代码审查所需要消耗的资源和代码审查这一流程的可持续性。

一个非常耗资源和可持续性很差的代码审查会让一个代码审查变得非常差劲。如果一个代码审查不是非常消耗资源,然后可持续性也很高的话,这会给审查者和受审查者双方都留下一个非常好的印象。

但是我们这里说的资源,和可持续性到底指什么呢?

资源

另一种来找出一个代码审查到底消耗了多少资源的方法是问自己这样的问题:谁在执行这个流程以及他们花了多少时间来执行这个流程?

大量的受访者们都是很有生产力的代码审查员,但是大家都表示对于团队里执行这个环节的人不爽,以及对于等待他们的代码被审查时花费的大量时间表示不爽。

下面是一些匿名的反馈:

我们有一个开发人员只是盲目的给每个PR一个赞然后都不怎么留评论。我可以告诉你这是真的因为他们 1 分钟能看 5 到 6 个 PR。

我发现第二个或者第三个的审查者大多数都是在打酱油。

有时候相同的代码审查的结果会根据不同的提交者而不同。

团队里的每个人应该受到相同的对待。高级工程师也会犯错,而且他们也希望有人能提出来。初级工程师也不能被各种黑。人呐,总是会被事物有偏见。

Commits 太长了,导致 PR 需要很久去审查。人们不会先在本地去测试一下代码。

长长的PR总是花费大量的时间,然后这个PR还对将来的特性有重要的影响。

各位码农对于代码审查和消耗的消耗资源的关系评论归类之后主要有以下三点:

  1. 大家都对代码审查不爽,而且觉得没什么卵用。
  2. 审查一个你不熟悉的代码很不爽。
  3. 错误都处在人身上,我们都是人。我们应该平等的对待,不能因为谁是老大就说他/她写的代码没有问题。

可持续性

代码审查可持续性主要被以下几个因素影响:执行代码审查的时候执行者到底说了什么,做了什么,让被审查者有什么样的感受

重点还是在于大家说了什么以及说话的方式。

让我们来看看大家的吐槽吧:

面对 PR 的时候,我觉得你要是不喜欢变量名就直接改,我觉得这个不是最重要的,这完全是个人喜好嘛!就像在 IDE 里一样,我很容易就被搞蒙了。我不关心为什么不开心,让这个东西停止报错就好,不停地报错真的不能忍。

不要在公开场合说思想上的大错误。在线下有个友善的对话会非常有帮助。直接 PR 上说会让人很不爽,最后弄得大家都不愉快。

当需求不停的改的时候我非常难过,特别是他们不向我解释为什么更改了需求的时候,或者留下他们犯错的可能性。特别是当别人告诉你改写你的代码成为他们的版本的时候,你简直难过得想要抱抱。

当一个回复过长的时候,我们不如去进行一次线下的交流。

我觉得对于个人喜好问题和功能是否能正常运作完全是两个问题。对于初级工程师来说,这是非常难分辨的。有时候几个高级工程师给出不同的反馈的时候更加懵逼。

总的来说,代码审查的重点有一下几个:

  1. 反馈过度注重语法和习惯的问题,导致让双方都非常不爽。代码习惯与风格和代码功能错误根本就是两回事儿。
  2. 说话方式也很重要。过激的语言会打消对方的自信心,对团队也不是好事。

如何做得更好?

也许这些数据不能最完整、最细致、或者最精准地代表代码审查的结构,但是我们还是能学到一些东西:我们能够回顾并检查我们团队的甚至整个社区的代码审查流程。

下面的匿名调查告诉了我们代码审查对于一个团队成员的影响是怎么样的:

一个非常差劲的代码审查让我几乎决定离职。一个优秀的代码审查流程会让我有信心面对将来更加困难的项目。

确实,有一个流程化的代码审查制度似乎是非常有效且能帮助团队快速成长的;Steve McConnell 和这篇文字的调查都证明了这一点。但是,单纯的重构代码审查流程然后永远不改是远远不够的。事实上,代码审查的流程应该根据整个团队的情况来更改以保证团队的生长。

与直接采用一个代码审查方式不同的是,我们需要重新思考我们的代码审查流程并且流程化工作来帮助我们面对以后的可能遇到的问题。

换句话说,重点在于我们能够思考我们的代码审查是否足够有效,是否能对于团队和个人同时产生好处。

一些提高代码审查的小技巧

这边是一些能够快速帮助提高代码审查感受的一些技巧:

  • 使用 linters 或者其他的代码分析工具来避免语法问题。
  • 使用 GitHub 的模板来生产每个 PR 。在发 PR 的时候带上更改列表对于作者和审查者都非常有帮助。
  • 在发 PR 的时候可以加个截图来帮助那些不是很熟悉的人理解问题。
  • 提高 commits 的信息量,尽量语言短小精湛。
  • 对于每个 PR 钦定审查者,如果可能的话人多于一个比较好。确定代码编写和审查的分配对于各个等级的工程师来说是均衡的。

很难做到但是非常重要的事情

当你已经代码审查入门之后,这里是一些更加重要的东西。事实上,对于重构代码审查流程来说,这里的东西是最重要的。

警告:前方高能,内容可能有一定难度

  • 能够理解你的团队。 这项任务一般都由高级工程师们来承担,希望大家对于新人们能多多帮助。

Markdown

  • 更加委婉的交流。 这意味着在发评论之前思考自己的言行是否妥当,是否会让其他人很不爽。在公众下批评别人是一件非常不好的事情。相比,有个私人对话会好很多。

Markdown

  • 进行一次口头交流。 带着整个团队坐下来,到 slack (美国的一种团队交流软件)上开个新的频道,让大家能够匿名的交流(选择最适合的即可)。在匿名的情况下大家都愿意说真话。

我把最重要的一点放在最后,因为当你还有耐心读到这里的时候,说明你真的想要更改一下你们的代码审查结构了,这是个好事儿。团队交流真的非常重要,这几乎是每个团队必须跨出的一步如果他们想要提升代码审查制度的话。

下面这句话基本上包括了我最想说的:

理论上来说,我很喜欢代码审查。事实上,这个东西取决于构建这个流程的团队。

相关资源

如果你想看一个更加大的匿名的反馈,你可以看下面这个有关于这个项目的网站:

Markdown

感谢

首先感谢一路上一直支持我的工程师朋友们,感谢你们投入时间和精力参与我的调查。






原文发布时间为:2017年6月05日

本文来自云栖社区合作伙伴掘金,了解相关信息可以关注掘金网站。

版权声明:本文内容由阿里云实名注册用户自发贡献,版权归原作者所有,阿里云开发者社区不拥有其著作权,亦不承担相应法律责任。具体规则请查看《阿里云开发者社区用户服务协议》和《阿里云开发者社区知识产权保护指引》。如果您发现本社区中有涉嫌抄袭的内容,填写侵权投诉表单进行举报,一经查实,本社区将立刻删除涉嫌侵权内容。

分享: