我们所不知道的Code Review

也许是待得太久,就像被一桶草莓酱从头浇到脚,尝哪里都是甜味一样,当初次看到Code Review成为一个如此重型并且低效的活动的时候,我才知道,草莓酱的外面,是空气,裹着大地的气息,大部分无味,又或者是烟草味,或者汽车的尾气。
先看一看我们看到的一个代码审查过程:
- 开发人员领到任务。
- 一周之后,代码写完了。他觉得没底,需要找业务专家技术专家来评审一下。这个时候他代码还没有提交。于是他把本地所有没有提交的、修改过后的文件,放到一起,压缩成rar包。找到他认为的技术高手业务高手(们),定会议室,发邮件通知。2小时过去了。
- 技术专家业务专家收到了邮件,由于缺乏上下文理解,以及长达数千行的源代码,这类邮件一般不看——因为看了也是白看。
- 终于到了Code Review的那一天。七七八八的来了几个人。一般来不全。因为高手之所以是高手,表现之一就是超级忙。
- 于是代码的作者开始,一行一行的将代码讲下去。前几十分钟高手们也没办法理解——毕竟是一个星期的工作沉淀,哪有那么快理解的。大约30分钟之后,专家开始提出建议意见。这些意见一般涵盖了语法、编码规范、可能的业务错误、模块间关系等。专家们毕竟是专家。2个小时之后,专家们离去。
- 开发人员虚心的把这些意见、建议写到小本子上。
- 开发人员可能根据专家的建议进行相应的代码修改并且提交,也可能不;可能改对了,也可能不。评审过后,后续的实施成为黑洞……
没了。
先思考一下,这个过程中的问题。
======== 思考的分割线 ========
首先必须承认Code Review的价值。经验丰富的专家们在做代码审查的时候,能够根据以往经验,规避重大缺陷的发生,对开发人员给予有价值的指导。然而,这个过程,太冗长,太低效。
- Code Review必须基于事实。这里的事实,就是,源代码库。SVN Repository, 或者HG/Git Repository. 在多人协作环境中,对于一份不在源代码库代码是基本不可信的 —— 你无法预知,他是否将会成为最终可工作软件的一部分。
- 积攒下来众多的代码修改,使得产生重型、低效的沟通方式成为必然。这类众型的沟通方式往往成本惊人 – 需要占用最好的人很长时间。
- 过分夸大专家的作用。根据以往经验,许多最终发现问题,回溯上来,其实是一些简单的逻辑问题。这些问题如果分散在平时结对或者更频繁的过程中,则更容易发现。很多情况下,是开发人员对常见的bad smell了解和修炼不足,而这些bad smell常常是导致问题的地方。例如在一个已经有3重循环的方法中加入了新的判断而没有测试;修改了函数的返回值而没有任何说明;if 判断中包含了多达4-5个变量的比较判断而没有抽象为一个更具业务含义的方法,等等。
- Review手段的原始落后。Review必须基于变化。会看报表的人都知道,看报表只需要看两个东西:趋势和拐点。Code Review也一样,只需要看变化。SVN/Hg/Git这类现代化的工具给我们提供了丰富的,基于changeset的compare工具。查看一天,整个团队的check in情况,顶多只需要10分钟-15分钟。
在敏捷过程中,Code Review几乎是一个被忽视的环节——不是不做,而是时时在做。结对时,我们会对结对伙伴的编码习惯、新写的类、变量表示质疑;提交之后,有代码静态检查工具、单元测试工具、覆盖率工具帮助我们检查有没有犯简单愚蠢的错误、有没有破坏既有功能;持续集成服务器则中立、不知疲倦的在每次我们提交之后运行所有的过程。
Code Review不是一个审查环节。不是一个考核环节。它是交流和反馈环节。

置身事外:能力与勇气

一台完美的晚会,演员为了观众而投入的表演,观众被表演而吸引,或捧腹或感动,而主持人一定是不温不火的那一个。他看起来不属于舞台:既不需要投入的表演,也不能忘我的欣赏节目,他最需要做的,就是按照既定的节奏,引导整台晚会逐步完成。观众笑,他可以笑但不能毫无顾忌;演员演得不尽人意,他通常也不能冲上去救场。他只能冷静的(无论从外表看起来他与观众、演员、嘉宾交流多么投入)按照时间规划,客观、严格的完成节目。
看起来,好像主持人是置身事外的。作为一台晚会,他没有参与其中最重要的部分。但从没有人质疑主持人的重要性,同时人们对这种分工也觉得很自然。但如果场面变一下:主持人跟观众一样感动得没了分寸,又或者觉得演员表演欠佳,他冲上去临时换着扮演,破坏演员节奏,恐怕最终结果不会那么乐观。劳动分工的精确含义在于,充分相信那个角色在当时场景下所做的努力,并尽最大努力帮助你的伙伴,不受干扰,节奏平稳的完成他的工作。
我现在依然看到,很多软件项目中,项目经理往往是一个悲情角色。他往往需要对整个项目负责,技术很强(开发背景),同时还要把握需求,控制需求,还需要跟客户(或者业务部门)交流,以及汇报。早些年参加软件工作的时候,我一直很羡慕项目经理的职位,呼风唤雨,加班(那时一直认为加班是一件很光荣的事情),随意抽调人,出差。后来有机会做了一些类似的工作,才发现这个职位几乎不能由人来胜任。项目经理要日常管理的同时,还需要是一个技术领袖,能够审核开发人员的代码。在这种情况下,项目经理一人之力撑起了一把伞,其他角色心安理得的躺在伞下,不用担心任何问题。
敏捷项目中,程序员更希望项目经理是一个看不见的角色。项目最终看起来是什么样子?由BA/QA决定。项目最终质量如何?由那帮写代码的决定。项目经理要做的,是置身事外。这里的事,就是具体的开发。冷眼旁观,但不评价,不插手,充分信任。置身事外需要勇气,即便在客户的高压下,也能够不加班的勇气;需要能力,需要辨别风险需求的能力。作为项目交付这台大的晚会,作为主持人,要做的就是保护你的程序员,是他们在任何情况下,能够持续稳定的产出可工作的、高质量代码。
勇气
作为极限编程中的原则(或者价值观)之一,在其他方面同样有效。项目经理要面对的最大的风险往往是需求的变更。作为一个正常的项目,大部分来自客户的变更都是有意义的。客户至上的理念在这里并不通用。在敏捷叠帽子游戏中,大部分的参与者都认识到时间与质量之间有一个平衡,总得拿些东西来换的。然而面对真正的客户,我们是否有勇气对一个看似合理的需求变更说不?说不很可能意味着客户不高兴,或者永久失去了一个客户。我们永远需要记住的是,客户需要100%,我们在规定时间内交付合格的哪怕50%都比交付不合格的99%要有意义得多。
能力
劝说客户是需要技巧的。与客户交流是需要技巧的。这些技巧程序员天生不具备。缜密的逻辑思维让他们不能在若干种模棱两可的商业外交语中如同编程代码一样游刃有余。这需要一个真正逻辑清楚并且沟通较好的人来完成。不要期待这样的人也能够写出比hello world级别更高的代码。通常情况下,他们只需要了解三层架构等等概念性的东西。与客户沟通,项目管理,分析项目开发进度,找到影响开发速度的地方并持续改进开发过程,适当激励才是项目经理需要做的事情。这些事情可以通过个人魅力,但也是需要技巧的。
置身事外
置身事外不是放纵。最大意味着,信任。一个团队中一定有一两个编程方面突出的人。保护这些激情并引导成为成就感,而不是架起一座大伞他们可以躺在伞下睡觉。不断强调整体团队与荣辱与共,这样项目的成功才能成为所有人的成功,而不是一个人的成功。
作为管理者或者推动者,是一个相当不能进入状态的角色,很多时候他们需要警惕破坏团队的因素并不断改进。上面的每一点都可以展开说去,每一点都是知易行难。我逐渐发现构建一个新的团队的过程远比引入新技术复杂的多。但不怕慢只怕站,持续改进,你的团队将成为最可怕、效率最高的团队。