敏捷实践——为什么代码评审如此重要
一: What 什么是代码评审
代码评审在开发过程中的一个通用的过程,传统意义的代码评审是大家一起通过会议等的方式进行代码审核,本文重点介绍的是基于Gerrit的代码同行评审。
现在,大部分项目基于Gerrit作为代码仓库,而Gerrit的基本功能,在于内置的同行评审流程。代码评审是Gerrit在提交的时候,直接使用评审的方式,Gerrit会直接发邮件通知相关评审人员,评审人员对代码进行评审和打分,只有打分达到要求的代码才能合入。
代码评审要求是小步提交,就是基于bug或story, feature粒度的提交,确保每次评审的代码都不多。 但在实际中,很多团队没有进行同行评审或评审效率不好。其原因主要是团队感觉安排人员进行评审浪费时间, 而评审人员因为自己任务紧张,不想花时间评审他人的代码。这一切的核心原因都是不理解为什么要进行同行评审和同行评审的意义。
二:Why 代码同行评审的作用
从个人,团队,和产品三个维度进行分析。
1: 对个人: 个人学习和成长的关键方式。
儒家强调“严己宽人”,但一般人都是审核他人的时候,会站在一个高标准,严要求的立场。 例如编码规范,让一个人去审核他人代码的时候,很容易发现很多不符合编码规范,Clean Code规范的内容,但自己在编写代码的时候,却很容易将这些忽略。所以,当我们用负责任的态度去评审他人的代码的时候,会发现他们的代码美妙的地方,也会发现他人代码丑陋的地方,而这本身会极大的提升自己的水平。 对比一下,如果一个人一直只关注自己的代码,很多东西会变得习以为然,而当其以评审员,以专家的角色对他人进行评审,其必须尝试以代码专家,业务专家的角度去思考和分析:这段代码有什么问题,我可以从那几个角度去发现这些代码中的问题。而这些思考,是一个人提升的必然步骤。 在敏捷中,我们的很多方式都是从上述角度出发的。例如:轮动SM,让每个成员都可以从SM的角度去考虑团队风险,进度。轮动会议主持人等。
2: 对团队: 人员建设和团队备份建设的高效方式。
在敏捷中,我们希望达到的目标是代码集体共享,但在实际操作中,不可能每个人对全部代码都非常熟悉。现在我们一般使用微服务的开发方式,一个产品维护多个服务,而前后的技术栈有较大区别,这些都给代码团队共享带来难度。但是,我们要达到的一个基本目的:每一个微服务都有大于等于2人完全掌握,严格避免一段代码,一个算法只有一个人掌握,一个人可以修改的“单点”模式。 假定我们设定微服务的”守护者“,那么守护者必须有2个人,才能在团队任务或人员出现异常的情况下保持产品的稳健。而这种最好的方式,就是代码评审。 通过代码评审,当该服务有其他人合入代码,但所有的代码”守护者“都全部了解和掌握。避免”全员负责“等于”没人负责“,避免”一人生病“,项目”全员等待“等问题。
3: 对产品: 版本质量的基本保证。
质量保证的2个基本逻辑是:
”质量必须全员保证“,
”问题发现越早修复成本越底”,
不包括需求偏差和架构缺陷,代码是故障的第一个阶段,所以代码评审是质量保证的一个核心环节。 假设代码存在逻辑缺陷,或基本错误,例如数组可能越界等, 如果测试在系统阶段测试,测试面对的这个版本,这个迭代提交的全部功能,发现一个函数中存在的问题存在很大的偶然,但是,对一个经验丰富的代码人员而言,这些问题在代码评审时候,可能如同夜里的萤火虫一样明显。 所以,代码审核对质量的保证可能远远大于系统测试(该例子使用kw等静态检查工具效果更好,仅仅用于举例)。更为核心的是:当代码审核过程很好执行时,当一个DEV了解到自己的代码将被展示和审核,其必然将对其更加用心。你能想象:一个准备宅在家里打游戏的男人和一个准备去和初恋美女约会的男人其装扮的差异吗?这和团队是否进行代码评审的代码质量差异基本等同。而整个团队的成员认识到产品的质量本质上控制在自己手中,由自己每天的一点点微小工作在保证而不是QA或测试的事情,这是项目质量提升的保证。
三:How 代码评审的层次
一个代码评审人员,以专家的身份:不仅仅是代码专家,更是业务专家,对他人的代码进行审核。这是每一个代码审核人应该把自己放的位置。一次代码评审可以从三个层次对代码进行审核:
1: 代码层次
代码层次从代码规范,Clean Code,代码故障等方面直接审核代码。可能包括 命名规范、 参数检查和限制(入参格式和正确性检查,出参统一)、 函数高内聚低耦合等原则(是否需要提取公共函数,函数是否很大)、 易发故障检查(数据是否长度检查,指针空指针)等, 总体就是从一个代码专家的角色考虑代码的设计原则、规范、安全性、性能等。
2: 业务层次
业务层次从业务的角度分析代码的实现是否合理,是否和需求的要求一致,同时考虑扩展性等方面。 对所有团队而言,交流/沟通都是一个核心问题,因为一个人想表达的和一个人实际表达的,以及另外一个人理解的可能都不一致。 敏捷团队运作就是为了解决该问题,但BA需求分析以后,DEV开发的可能有DEV理解上的偏差,这个需要评审人员从业务的角度考虑代码实现的合理性,最优性,和扩展性。 总体就是从一个业务专家的角色考虑代码和需求的一致性。
3: 架构层次
代码评审必须要先进去,然后还要出来。进去看代码的函数、代码的变量、代码的逻辑。出来看代码的业务,更要看新增代码和原有代码的关系。 这需要评审员对整个系统的代码都非常熟悉。从整个系统,整个架构的角度看新增的代码在结构中是否合适。新增的代码是否和原有的代码存在某种可以提取的公共函数?这个地方修改是否意味另外一个地方必须修改?是不是影响接口,是不是影响外部系统,是不是影响升级。 总体就是从架构、从整体的角度分析新增代码对原有系统的影响。
代码评审是DevOps中最核心的一个质量门,对人员培养和团队建设都有重要的意义,所以,做、坚持做、持续做是我们对其唯一正确的态度。