外包公司如何做代码审查

[LipYoung]

概述

外包公司,指的是面向企业或者个人,承包软件应用开发的公司.代码审查的本意是在编码初期系统化的审查代码,常见的手段的是同行审查,结对编程等.目的是找出并且修正软件开发初期出现的错误,提高质量.

软件编码初期发现问题进行修复的成本要软件末期(交付用户手动测试)时修复成本低近千倍.

代码审查可以找出,内存泄漏,缓存溢出等问题,多数时候采用 Git 方便同行审查代码.也包括自动化审查工具这种非正式的方式检查代码缺陷.

外包公司代码审核的困境

但是外包公司诸多特点导致不利于展开代码审查:

1. 外包项目业务代码比例大.
   在整个项目中,业务代码可能超过了 80%.业务代码指的是有个性的定制话的单一功能,较难重用.审核业务代码需要熟悉相关的功能需求等内容,审核成本过高.
2. 外包项目成员组成单一
   大部分外包项目以交付为主,团队内部只有1-2名熟悉某种单一开发环境开发人员,开发水平参差不齐,所以导致无人审核代码.
3. 外包项目管理模式
   大部分外包项目都会面临甲方不懂开发,产品经理不懂开发,甚至团队管理不懂开发的情况,团队没有合理的针对开发的KPI考核方式,开发成果也单一的以用户手动使用软件的结果来评审.这会导致代码审核这一过程显得多余且无效,直至代码审查被抛弃.
   同时因为外包项目开发时间紧凑,开发团队中部分有能力的开发人员每天疲于应付业务代码的开发,没有时间每天审查代码,指出问题;看到质量低的代码直接动手就给他重新编写了一遍.

常规的代码审查方式

一般的的审查方式是逐行阅读代码,普通的代码一小时阅读200行左右,对于核心代码该速度过快.
通过 Git 的分支流模式( PR )可以极大的方便对代码的审核.这里介绍一种简单的常用的分支流程:

一名开发创建单独分支进行编码,编码完毕后提交到远端.另一名开发进行审核,提交出现的问题,驳回或者通过审查进行合并.

如果整个过程使用 Github 的话,会生成一个页面,方便大家对于本次提交进行讨论.

在外包项目开发的过程中,因为时间等原因,大家会感觉这样显得过于繁琐.甚至有的开发人员会直接重写待审核的代码.

但是不推荐这样做,从被审查者的角度来讲,代码质量低可能因为对业务不熟悉,语言特性不了解,也可能是不了解整个项目架构.如果你直接帮助他修改了代码而不是指出问题所在,下次他编写的代码质量并不会提高.不仅无异于他人的成长,也会导致深深的挫败感.还有就是从长远来看,审查5个人的代码,一次比一次出的问题少,直至不需要审查,让他们自己相互审查,通过这种方式带新人,相当于复制了5倍你的开发能力,对比1个人写5个人的代码来说,优势显而易见.

综上所述,审查是必须有的不能少,那么如何兼顾外包紧张的时间呢?

适合外包公司的方式

引入自动化审查工具

代码审查主要检查的方面包括:1.代码风格和可读性检查. 2.错误处理覆盖. 3.边界情况处理. 4.文件内的代码结构. 5.类的组织架构.
第一点和第四点都可以通过引入自动化审查工具来解决.我团队在开发ThinkSNS+的过程中,iOS端就使用了 Swiftlint 工具.可以在每次开发者 build 项目时进行细致的检查.小到单个文件源码行数,大到不允许强制解包(swift语法特性),不允许出现复杂度超过10的函数等方方面面.

这样可以起到低成本高效率的统一工程的代码风格和提高部分代码质量.剩余要检查的部分可以通过审查会议处理.

每个周期开一次审查会议

为了在时间紧迫的情况下也进行代码审查,折中的方式就是每个周期(大项目24个工作日一次,小项目 10个工作日一次),开一次集体的面对全体开发同事的审查会议.
通过待审查人员的提前准备将会议控制在1个半小时内.同一种开发的同事一起参加会议,让大家一起改进代码质量.

会议中有针对性的审查错误的覆盖情况,代码文件内和文件外的组织架构,错误处理情况等.这种会议很容易出现带有情绪话的审核,感觉指出问题是得罪人,不明示发现问题的情况,抱团,表面上看起来和和气气的情况.所以一定要团队领头人带头组织会议,预先准备好要讲的有针对性的错误情况和内容.始终记得代码审查是工作,就事论事的做好工作.

结对编程

结对编程也是我在实践中发现的非常好的轻量级的代码审查方式.实践后的结对编程方式和原本的定义有所差别.我司在外包项目中,总是会开发移动端,而移动端又分为iOS端和Android端,正常情况下,移动端的开发进度总是相同的.此时,我会将2个端的开发人员的工位在一起,并且鼓励他们一起讨论开发中的细节,错误处理,问题等.实践证明,虽然2个端的语法不一样,但是因为不会逐行进行详细的代码查阅,且因为开发业务一致,大量细节处理(例如断网情况,用户暴力输入情况)和问题都会在开发前期就被发现并且解决.

总结

当然这种轻量型代码审查方式所找到的缺陷会比正式的代码审查要少,但其速度较快,其成本也较低.是较为契合外包公司的方式.如果条件许可的话,肯定不如每次产生PR 都进行想起的审核来的效果明显和直接.