引言
代码CR(Code Review)是软件研发活动中保障平台产品质量的重要环节,相信很多技术团队平常都会进行代码CR。就拿阿里来说,一般周二和周四都是发布日,那么在发布上线某项功能之前都要组织进行发布代码CR,CR不通过的代码必须修改检查通过后才能发布上线,可见一线互联网大厂技术团队对于代码CR的重视程度。虽然大家对于代码CR都不陌生,但是在自己团队中实际落地的时候不免还是会遇到这样或者那样的问题,比较典型的问题有如下几种:
1、到底是所有的代码都需要进行CR,还是只要核心业务代码才需要进行CR?
2、怎样才能让reviwer能够认真评审代码?
3、线上CR还是线下CR?
4、代码CR很费时间和精力,如何才能保证在花费的时间和精力后可以达到预期效果?
本文就和大家探讨下到底怎么做才能在技术团队中高效的落地代码CR活动,而不至于最后导致代码评审活动流于形式只是走个流程而已。
为什么要进行代码CR
提升团队代码水平
一般技术团队都会由不同技术水平以及不同工作年限的同学组成,而Code Review是非常好的大家在一起互相学习代码设计的机会,因为在Code Reiew过程中不仅仅会检查业务代码逻辑问题,还包含了结构设计、程序性能、代码安全、程序鲁棒性等等方面综合性检查。因此团队中的核心骨干如果能够重度参与代码CR活动中,不仅在一定程度上可以帮助其他同学的成长,同时也可以协助新同学快速融入团队。
另外在编码的时候,代码CR机制会像一只无形的手,不断鞭策着团队成员不要写烂代码。因为大家知道自己写的代码需要在团队内部进行公开的CR,这样压力就会油然而生,它会促进自己不要写烂代码,因为每个人也不希望自己写的烂代码曝光在大家面前,这样自己面子上也挂不住。因此CR机制在一定程度上可以让团队同学避免写一些短期收益的代码,从而欠下技术债留给未来维护同学。
CodeReview不是人情世故,而是程序员的技术炼金场。
确保设计实现一致
在程序猿的日常工作中,通常在业务方需求KO之后会进行对应需求的设计和实现。但是在实际的研发活动中,经常会出现实现和设计之间存在一定的偏差gap,而这些gap可能会导致后期上线的Bug以及代码维护问题,因此在代码CR的时候就要重点关注设计与代码实际实现之间存在差异问题,尤其是需求Owner要重点review业务-》设计》实现的一致性。
统一团队编码规范
在实际的代码CR过程中,经验丰富的老司机分别会从命名、代码结构设计、程序性能等方面进行分析。其实同样一个需求,让100个程序员来做,写出来的代码可能有100种样子,我觉得这很正常。实现逻辑可以五花八门,但是在一些比较通用的编码规范层面,需要保持统一。
如果有这样的业务场景,你定义了一个订单的DTO类,其中有个字段是订单的审核状态,假设有待审核、审核通过、审核不通过三种状态,我们通过定义枚举的方式来表示不同的状态。这里的inspectStatus分别对应枚举中的三个状态属性。大家觉得下面的代码有什么问题?
public class OrderDTO {
...
/**
*审核状态
*/
private Integer inspectStatus
...
}
实际上并不是代码本身有什么问题(都是属性能有啥问题),而是在可读性方面存在问题。假设你是刚接手负责这个模块,当你看到这个定义的时候,是不是迫切想知道到底这个审核状态有哪几种,但是由于代码不熟悉,你并不知道去哪里寻找,因此在可读性以及可维护性方面不尽如人意。
遇到这种情况我们可以在审核状态字段的注释上面加上一个@link,可以直接链接到对应的状态枚举类,这样后来维护业务的同事可以通过实体类直接查看到订单的各个审核状态,总比自己无头苍蝇的在工程中找或者问其他组内同事来的效率高。因此我们在编写代码的时候不仅要考量如何实现当前的需求,也要想着如果未来别人来维护我写的代码,那么怎样才能让后续维护的同学更好更快的掌握业务逻辑。
public class OrderDTO {
...
/**
*审核状态
*{@link com.mufeng.eshop.biz.order.InspectStatusEnum}
*/
private Integer inspectStatus
...
}
这里举了个看似简单的例子,但是在实际的编码中却十分常见,因此需要对团队中的代码规范进行统一的规定。代码规范性层面可以参考《阿里巴巴Java开发手册》,另外Idea有对应的插件Alibaba Java Coding Guidelines。
厘清业务逻辑细节
一般一个技术团队可能会负责多条不同的业务线。这些业务可能都是存在一定的关联关系的。但是平时同学们都在忙于应付各种各样的业务方需求,大家可能没有太关注同组同学所负责的业务的具体逻辑细节,因此通过代码CR可以让大家有机会了解各个业务线的逻辑细节,这样更加便于团队成员厘清上下游的业务逻辑,将来涉及到完整业务链业务需求的时候,在进行设计的时候可以考虑的更加全面以及识别一些关键设计点。
如何保证代码CR效果
如果我们想要保证代码CR的落地效果,我们就需要搞清楚到底哪些因素会影响技术团队代码CR效果。这里大致总结了日常工作中影响代码CR效果的因素:
对于提交代码评审的同学:
1、不清楚提交代码CR的范围;
2、不清楚需要给哪些人提交代码CR;
3、怎样才能让别人认真评审代码;
对于评审别人代码的同学:
1、不清楚需要CR代码的业务上下文是怎样的,不容易判断代码结构设计的合理性;
2、一下子提交几千行代码,哪些代码是CR的重点内容,哪些不是;
上述问题都是制约技术团队代码CR落地效果最常见的问题,我们到底应该怎么解决这额问题呢?
线上评审结合线下评审
线上评审一般是主要的代码CR方式,但是在提交评审的时候还是要遵循一定的原则,以便于提高代码评审的效率。
1、每次提交CR的代码不能过多,如果每次评审的时候一下子推给别人几千行代码,估计对应的reviwer看都不想看,很难保证review的质量;
2、在提交评审的时候,需要附上一定的说明,阐述清楚这些代码主要实现什么样的业务需求,主要核心逻辑在哪些文件中,这样reviwer在评审代码的时候可以有的放矢。
线下评审作为线上评审的重要组成部分,比如一周一到两次的线下会议评审一般可以满足需要。在线上评审之前最好先和reviwer敲定好时间以及需要评审的代码,提前准备好需要CR的代码背景,比如说对应具体的需求是什么,自己在进行代码落地的时候是如何分析问题的,大致的类结构是如何设计的等等,这样reviewer们可以比较清晰的理解代码的背景。
另外线下评审的代码量不要过大,时间尽量保持在一个小时左右。评审会议的时候要记录大家提出来的建议以及问题,会后修改后再和大家对焦确认。
找到合适的代码评审者
将代码提交给合适的Reviewer进行评审这一点非常重要,因为如果将代码提交给了没什么业务关联的或者和自己技术水平差不多的Reviewer,一方面业务不相关的同学很难理解其中的业务规则代码看起来也费时又费力,另一方面水平相似无法高屋建瓴的提出来改进意见,因此基本很难获得比较好的review结果。
1、向团队中经验丰富的程序员提交CR,以便于获得更加高水平的代码设计反馈;
2、向业务需求Owner提交CR,需求Owner一般对于这部分的业务需求非常熟悉,因此可以从业务层面或者技术层面给出更好的意见;
3、向修改过相同文件的同学提交CR,这样大家彼此知道自己的修改意图以及原因,便于评估影响范围。
建立评审奖励机制
或许是因为大家平时工作太忙,或许是因为提交给了不合适的reviewer。我们总是担心别人到底有没有认真CR我们的代码,那我们到底该怎么激发大家认真review代码呢?这里提供一个可落地实操的机制,比如我们可以在团队内部建立明确的代码CR奖励机制,对于在代码CR过程中评审出来的高质量Bug的reviewer进行奖励(奖牌或者奖杯都可以,同时在P8下面的大团队中进行通报表扬提升影响力)。每个月评选捉虫高手,注重数量更加注重质量,通过这样的奖励机制来正向引导大家去积极进行代码CR。
不过这里面存在一个隐含的Bug,就是如果团队中有一个技术大牛,那么大家可能都会把代码提交给他来审核,那么技术大牛代码评审量就会非常大,对于大牛来说就变成甜蜜的负担了,因此还是要有所取舍,比较核心重要的代码再提交给技术大牛进行评审,这样既保证了核心业务逻辑代码的评审权威性,也不会给团队中资深工程师的工作负担。