目标和原则
- 提高代码质量,及早发现潜在缺陷,降低修改/弥补缺陷的成本
- 促进团队内部知识共享,提高团队整体水平
- 评审过程对于评审人员来说,也是一种思路重构的过程,帮助更多的人理解系统
- 是一个传递知识的手段,可以让其它并不熟悉代码的人知道作者的意图和想法,从而可以在以后轻松维护代码
- 可以被用来确认自己的设计和实现是一个清楚和简单的
- 鼓励相互学习对方的长处和优点
- 高效迅速完成Code Review
流程和规则
采用Git Flow + Pull Request(PR)模式来做Code Review。
Git Flow
Pull Request(PR)
Pull Request 的说明
- 任务完成才能提交PR
- 严禁一个PR里面有多个任务,除非它们是紧密关联的
- PR提交之后只允许针对Review发现问题再次提交代码,除非有充足的理由,严禁在同一个PR中再次提交其它任务的代码
- 提交PR时候有一个描述框,内容会自动根据Commit的message合并而成。切记,如果一次提交的内容包含很多Commit,请不要使用自动生成的描述。请用简短但是足够说明问题的语言(理想是控制在3句话之内)来描述: 你改动了什么,解决了什么问题,需要代码审查的人留意那些影响比较大的改动。特别需要留意,如果对基础、公共的组件进行了改动,一定要另起一行特别说明。
- PR应该在1~2个工作日内被合并或者被拒绝
- 发起Pull Request以后,请将Pull Request的链接通过Email发给代码审核者,以此通知对方及时进行审核。(BUG修复类当日必须完成合并或者拒绝,功能类或者觉得有重大调整需要会议Review必须在邮件中明确时间和会议人员)
Code Review Checklist
Code Review主要检查代码中是否存在以下方面问题:
代码的一致性、编码风格、代码的安全问题、代码冗余、是否正确设计以满足需求(功能、性能)等等
-
完整性检查
- 代码是否完全实现了设计文档中提出的功能需求
- 代码是否已按照设计文档进行了集成和Debug
- 代码是否已创建了需要的数据库,包括正确的初始化数据
- 代码中是否存在任何没有定义或没有引用到的变量、常数或数据类型
-
一致性检查
- 代码的逻辑是否符合设计文档
- 代码中使用的格式、符号、结构等风格是否保持一致
-
正确性检查
- 代码是否符合制定的标准
- 所有的变量都被正确定义和使用
- 所有的注释都是准确的
- 所有的程序调用都使用了正确的参数个数
-
可修改性检查
- 代码涉及到的常量是否易于修改(如使用配置、定义为类常量、使用专门的常量类等)
- 代码中是否包含了交叉说明或数据字典,以描述程序是如何对变量和常量进行访问的
- 代码是否只有一个出口和一个入口(严重的异常处理除外)
-
可预测性检查
- 代码所用的开发语言是否具有定义良好的语法和语义
- 是否代码避免了依赖于开发语言缺省提供的功能
- 代码是否无意中陷入了死循环
- 代码是否是否避免了无穷递归
-
健壮性检查
- 异常处理和清理(释放)资源
- 代码是否采取措施避免运行时错误(如数组边界溢出、被零除、值越界、堆栈溢出等)
-
结构性检查
- 程序的每个功能是否都作为一个可辩识的代码块存在
- 循环是否只有一个入口
-
可追溯性检查
- 代码是否对每个程序进行了唯一标识
- 是否有一个交叉引用的框架可以用来在代码和开发文档之间相互对应
- 代码是否包括一个修订历史记录,记录中对代码的修改和原因都有记录
- 是否所有的安全功能都有标识
-
可理解性检查
- 注释是否足够清晰的描述每个子程序
- 是否使用到不明确或不必要的复杂代码,它们是否被清楚的注释
- 使用一些统一的格式化技巧(如缩进、空白等)用来增强代码的清晰度
- 是否在定义命名规则时采用了便于记忆,反映类型等方法
- 每个变量都定义了合法的取值范围
- 代码中的算法是否符合开发文档中描述的数学模型
-
可验证性检查
- 代码中的实现技术是否便于测试
-
可重用性
- DRY(Do not Repeat Yourself)原则:同一代码不应该重复两次以上
- 考虑可重用的服务,功能和组件
- 考虑通用函数和类
-
可扩展性
- 轻松添加功能,对现有代码进行最小的更改。一个组件可以被更好的组件替换
-
安全性
- 进行身份验证,授权,输入数据验证,避免诸如SQL注入和跨站脚本(XSS)等安全威胁 ,加密敏感数据(密码,信用卡信息等)
-
性能
- 使用合适的数据类型,例如StringBuilder,通用集合类
- 懒加载,异步和并行处理
- 缓存和会话/应用程序数据
代码检查包括不局限于上述清单,提交人应在本地自我完成代码格式、架构设计、面向对象分析与设计等检查。
备注:
- Java服务端开发必须遵循阿里巴巴Java开发手册(终极版),IDEA中安装相关插件有告警提示不允许提交合并
Code Review的步骤
- 代码编写者和代码审核者坐在一起,由代码编写者按照UC依次讲解自己负责的代码和相关逻辑,从Web层->DAO层;
- 代码审核者在此过程中可以随时提出自己的疑问,同时积极发现隐藏的bug;对这些bug记录在案。
- 代码讲解完毕后,代码审核者给自己安排几个小时再对代码审核一遍。代码需要一行一行静下心看。同时代码又要全面的看,以确保代码整体上设计优良。
- 代码审核者根据审核的结果编写“代码审核报告”,“审核报告”中记录发现的问题及修改建议,然后把“审核报告”发送给相关人员。
- 代码编写者根据“代码审核报告”给出的修改意见,修改好代码,有不清楚的地方可积极向代码审核者提出。
- 代码编写者 bug fix完毕之后给出反馈。
- 代码审核者把Code Review中发现的有价值的问题更新到"代码审核规范"的文档中,对于特别值得提醒的问题可群发email给所有技术人员。
备注:
Code Review必备的文档:
“Code Review规范”文档:记录代码应该遵循的标准。
代码审核者根据这些标准来Code Review代码,同时在Code Review过程中不断完善该文档。
Code Review的执行
准备阶段
- 评审规范和标准
- 在CR前设计确定评审规范和标准是必要,通过规范和标准我们在审查过程中可以有据可依,有理可循,而且还可以做到标准统一。
- 选择CR活动的参与者
- 在CR开始前,必须把本次CR活动的对象、审查内容以及审查的规范和标准通过email通报给所有的参与者。
- 选择CR活动的实施方式
- CR活动有很多形式可供我们选择,我们可以根据实际情况选择桌面式CR、演示讲解式CR、一对一的座位CR等等。(一般按新增功能桌面式CR、里程碑功能演示讲解式CR、BUG修复一对一的座位CR)
实施阶段
充分的事前准备,只是做好CR活动的前提,在CR实施过程中,我们要做好以下工作。
- 准确记录
- 对于CR过程发现的问题,我们必须清晰准确的记录,可以使用问题点记录单,明确记录的项目和内容。
- 讲解与提问
- CR过程中,要采用代码作者讲解和审查者提问方式。审查者不能只在发现问题时提问,同时也要根据本次审查的内容要求代码作者对某个特定问题的讲解。
- 逐项审查
- 对事前确定的审查内容,要逐项审查,不能因为时间不足等因素一扫而过。
- 注意气氛
- 实施审查时,要营造一个讨论问题、解决问题的氛围,不能把审查会搞成批判会,这样会影响相关人员的积极性。
事后跟踪
- 确认发现的问题 CR结束后,对发现的问题,首先需要确定以下内容。
- 问题点的难易程度以及影响的范围;
- 解决问题的责任者和问题点修正结果的确认者;
- 解决问题点的时限。
- 修正问题责任者 对于修正问题责任者,在问题点的修正过程中,要三方面内容的记录。问题点的原因;
- 解决问题点的对策;
- 修正的内容。
修正结果确认者做为修正结果的确认者,必须按照事前约定的时限及时的对修正结果进行全面的确认
注意事项
- 经常进行Code Review
(1)要Review的代码越多,那么要重构,重写的代码就会越多。而越不被程序作者接受的建议也会越多,唾沫口水战也会越多。
(2)程序员代码写得时候越长,程序员就会在代码中加入越来越多的个人的东西。
(3)越接近软件发布的最终期限,代码也就不能改得太多。
- Code Review不要太正式,而且要短
忘了那个代码评审的Checklist吧,走到你的同事座位跟前,像请师父一样请他坐到你的电脑面前,然后,花5分钟给他讲讲你的代码,给他另外一个5分钟让他给你的代码提提意见,这比什么都好。而如果你用了一个Checklist,让这个事情表现得很正式的话,下面两件事中必有一件事会发生:
(1)只有在Checklist上存在的东西才会被Review。
(2)Code Reviews 变成了一种礼节性的东西,你的同事会装做很关心你的代码,但其实他心里想着尽快地离开你。
只有不正式的Code Review才会让你和评审者放轻松,人只有放松了,才会表现得很真实,很真诚。记住Review只不过是一种形式,而只有在相互信任中通过相互的讨论得到了有意义和有建设性的建议和意见,那才是最实在的。不然,作者和评审者的关系就会变成小偷和警察的关系。
- 尽可能的让不同的人Review你的代码
如果可能的话,不要总是只找一个人来Review你的代码,不同的人有不同的思考方式,有不同的见解,所以,不同的人可以全面的从各个方面评论你的代码。
但不要太多了,人多嘴杂反而适得其反,基本上来说,不要超过3个人,这是因为,这是一个可以围在一起讨论的最大人员尺寸。
下面是几个优点:
(1)从不同的方向评审代码总是好的。
(2)会有更多的人帮你在日后维护你的代码。
(3)这也是一个增加团队凝聚力的方法。
- 保持积极的正面的态度
程序员最大的问题就是“自负”,尤其当我们Review别人的代码的时候,我已经见过无数的场面,程序员在Code Review的时候,开始抨击别人的代码,质疑别人的能力。太可笑了,我分析了一下,这类的程序员其实并没有什么本事,因为他们指责对方的目的是想告诉大家自己有多么的牛,靠这种手段来表现自己的程序员,其实是就是传说中所说的“半瓶水”。
所以,无论是代码作者,还是评审者,都需要一种积极向上的正面的态度,作者需要能够虚心接受别人的建议,因为别人的建议是为了让你做得更好;评审者也需要以一种积极的正面的态度向作者提意见,因为那是和你在一个战壕里的战友。记住,你不是一段代码,你是一个人!
- 学会享受Code Review
这可能是最重要的一个提示了,如果你到了一个人人都喜欢Code Review的团阿,那么,你会进入到一个生机勃勃的地方,在那里,每个人都能写出质量非常好的代码,在那里,你不需要经理的管理,团队会自适应一切变化,他们相互学习,相互帮助,不仅仅是写出好的代码,而且团队和其中的每个人都会自动进化,最关键的是,这个是一个团队。