技术交流

CodeReview是什么呢?
北大青鸟市场部

摘要:CodeReview是什么呢?

CodeReview是什么呢?字面意思,就是对你的代码进行评审,talkis cheap,showme the code就是这意思。越来越多的企业都要求研发团队在代码的开发过程中要进行CodeReview(简称CR),在保障代码质量的同时,促进团队成员之间的交流,提交代码水平。
不过CR文化的推崇却是近些年才开始的,国外像Amazon、Google的大厂要求代码合并进主干分支时必须要做CR,国内互联网起步比国外晚,再加上略微复杂的中国特色(不太好意思当面评论别人的代码写的不好….),早期的CR更多流于形式,现在随着互联网流量的剧增,必须要保障代码质量才能保障业务高稳定高可用,所以CR开始成为企业开发中必须要做的事情。
简单来说,CR就像吃早餐一样,不吃好像不见得对身体会有什么不好,所以大家因为工作忙或睡懒觉等原因,忽略了早餐,或者随便吃吃应付。
但随着身体在高负荷、高压力的情况下逐渐有些异样,大家开始意识到了“身体是革命的本钱”,早餐必须要吃,必须坚持吃,这样对身体是最有益的,身体好才有力气走的更远、做的更好。不过早餐为什么吃,什么时候吃,怎么吃,吃什么也是有考究的。就像CR什么时候做,怎么做,该注意什么也是有考究的。


Why?--为什么需要评审?
CR是代码规范性的保障、带来知识传播、团队建设。有的人可能觉得代码评审就是找出错误的,觉得代码评审没有必要,实际上并不是这样。
从编码者的角度来说,每天都忙于紧张的coding,交付时间马上到了,为了快速交付于是降低了要求,不写单元测试用例,coding的时候也没有从性能、安全角度去考虑更好的实现,虽然按期提交了代码,然而却不是一份高质量的代码,在运行中可能出问题、后来人接手也不好接。
如果有CR的存在,想到你的代码即将被你的同事、领导进行评审,你还会降低要求吗?肯定加班加点保质保量的完成代码编写呀。
另外在CR的过程中有资深的前辈对你的代码设计思路、算法进行讲解,这肯定比自己低头琢磨进步快啊。
CR评审的理由就像和家人朋友一起吃早餐一样,一个人的时候起晚了或者着急就不吃了,而如果你家人或朋友与你一起居住时,一想到家人的健康、家人对你的担忧,你还会不早点起床吃早餐、甚至做早餐吗?
When?—什么时候评审?
每一次的代码合并(PullRequest/Merge Request)就是最好的时机。PullRequest就是说你没有权限往一个特定的仓库或分支提交你写的代码,通过请求有权限的人将你提交的代码从你的源仓库的源分支合并进目标仓库的目标分支。
每个需求的改动都应当尽快提合并请求合并到主分支,这样可以尽早的发现代码编写中的问题。我们现在所倡导的持续集成也是这个思想,不要等到所有的需求都开发完在进行合并,一次提交大量的代码给评审的人也带来很大负担,修改一次提交一次。当然这个提交也是要有质量的提交,至少在提交之前自己已经全面review、通过了单元测试。
CR评审的时间就像做早餐一样,必须自己都尝试生熟合适、甜咸得当再叫别的人一起来吃。
How?—怎么评审?
选择合适的工具、配合合适的开发流程、选取适当的形式这三者非常重要。
对于工具来说,目前很多代码托管工具如Github、Gitlab、阿里云云效、腾讯工蜂等都自带了CR工具,开发团队可根据自己情况选择。
对于开发流程,目前流行的GitFlow、主干开发模式、fork开发模式都支持在将代码合并到master分支时需要发起PullRequest/MergeRequest。对于适当的形式,包含线上评审、线下评审、特殊处理三种,对于轻量级的CR(比如小功能模块的开发、不超过500行的代码)可直接在代码托管工具中邀请同组的人或资深的人对代码进行评审,结合反馈意见进行修改;对于大功能模块的开发或是涉及架构变动,可组织团队人员线下进行评审,开发者讲自己的设计逻辑,评审人给出意见,一行一行的进行代码评审;对于某些紧急情况,比如线上有紧急bug需紧急上线但又没有人在,这时候可以进行紧急合并,但事后仍然需要补上CR。
CR的评审方法就像吃早餐一样,如果是一个人可以简单一些,牛奶面包补充必要的蛋白质即可;如果是当一家人在一起时,必然会丰盛一些,包子、粥、豆浆、油条、咸菜都会来一点,五谷杂粮都进行补充;如果是紧急赶火车或赶飞机来不及吃时,可以先不吃,等上了火车或飞机再补上。
What?—评审什么?
CR评审什么呢?在CR中我们对代码的规范性、一致性、编码风格、代码的安全问题、代码冗余、代码的功能性能设计等进行评审。
对于规范性,在java中我们会去check后台线程是否有同步访问主线程、函数及变量命名是否准确、组件分层是否合理,公共逻辑是否合理抽出,文件组织是否合理、函数注释是否清晰全面、代码的可读性是否良好,是否有更优雅的写法、程序设计是否满足单一原则,开放封闭原则。
对于完整性,我们check代码是否完全实现了设计文档中提出的功能需求、是否创建了需要的数据库、是否包含正确的初始化数据。
对于正确性,我们check是否所有的变量都被正确定义和使用,是否有未定义的变量被使用、是否有明显的或潜在的逻辑bug、是否无意中陷入了死循环、是否避免了无穷递归。
对于健壮性,我们check代码是否做了异常处理,是否存在数组塌陷、内存溢出。
对于可重用性,我们check组件是否可复用、代码是否存在重复。
对于可扩展性,我们check功能组件是否便于扩展、代码是否可以下沉复用。
对于安全性,我们check是否进行身份验证,授权,输入数据验证,避免诸如SQL注入和跨站脚本(XSS)等安全威胁,加密敏感数据(密码,信用卡信息等)、引入的依赖项是否安全,成熟、公共组件&工具函数的改动,是否会影响其他业务。可见CR并不是一件简单的事情,一份好的代码、好的工程师也必定是受过千锤百炼。
CR的评审内容就像早餐一样,我们会注意食材营养搭配是否均衡、烹饪是否得当、量是否足够、食材是否安全、是否清洗干净、就餐环境是否干净卫生、价格是否合适等等。
不要说业务迭代太多、需求太多、上线时间紧张没有时间做CR,不要为自己丑陋的代码找华丽的借口,没有时间好好做CR,那将有大量的时间用于焦头烂额的处理故障和投诉。
就像不要说自己没时间吃早餐、工作太忙、太困一样,现在省的时间将来有的是各种病让你浑身不舒服。
所以如果你在的团队还没有做CR、CR践行的不好,一定要push你的leader找到根本的原因,将CR践行下去,为了一份有质量的代码,一切都是值得的。
就像如果你现在还没有好好的吃早餐,从现在开始好好的吃早餐,为了一个健康的身体,一切都是值得的。


相关阅读
热门推荐