朱赟的技术管理课_33_32_硅谷人如何做_Code_Review
你好,我是朱茵。
今天我分享的主题是硅谷人如何做code review.今天技术管理课的主题是code review,也就是我们常说的代码评审。
Code review主要是在软件开发的过程中对源代码进行同级评审,其目的是找出并修正软件开发过程中出现的错误,保证软件质量,提高开发者自身水平。
和国内的工程师聊天,发现国内公司做代码评审的比例并不算高,这可能和各公司的工程师文化有关系。
不过,硅谷稍具规模的公司,代码评审的流程都是比较规范的,模式也差不多。
首先是两个概念,在进入正题之前,先介绍两个概念,一个是commit,一个是PR.硅谷大部分公司都使用github企业版管理自己的代码仓库。
Github里的commit和PR的概念在代码审核中非常重要。
一、commit,也就是github上的一次commit行为,这是可以单独保存的源代码的最小改动单位。
二PR,也就是poor request是一次代码提交请求,一次PR可以包含一次commit,也可以是多个提交请求后,github会在相关页面上显示这次提交请求的代码和源代码的所有不同之处。
这就是本次PR的所有改动。
请求。
提交后,其他工程师可以在PR的页面上提出意见和建议,也可以针对某一些代码的改动进行讨论,也可以给整体评价代码的作者也可以回复这些意见和建议,或者按照建议进行改动。
新的改动将为本次PR中提交新的commit.关于github和poor request,曲老师最近在他的公众号里写了一篇github为编程世界带来了什么?改变。
这篇文章中有比较详细的描述,你可以参考学习。
其次,我来谈谈代码合并规则。
一般情况下,所有的PR都必须有,至少一个人认可才能进行合并。
如果改动的内容涉及多个项目,则需要每个项目都有相关人员认可才能合并。
还有一些特别关键的代码,比如支付相关的,通常也会需要支付组的人确认才行。
在代码合并之前,进行code review的工程师们会在githup的相关页上给出各种评论。
页面是共享的。
这些信息大家都能看到,有些评论是询问代码的作者直接回复或解释就行。
有些是指出代码的问题,代码作者可能会据此改动,也可能不会同意,那就需要回复评论,阐述观点,你来我往。
有时候,一个实现细节讨论的主题可以多达十几条或几十条,最终需要达成一致,才能进行合并。
在此帮助别人成长,而不是帮他写代码。
以前有朋友会说,看到代码写的太差,觉得来回评论效率太低,干脆自己冲上去搞定。
曾经我也有过类似的想法,不过我慢慢意识到,这并不是一个合适的想法。
首先,从对方的角度来说,代码写不好可能是对业务不熟悉,对编程语言不熟悉,也可能是对公司代码的整体架构不熟悉。
如果你帮他写,而不是耐心指出哪里有问题,那么下一次他可能还是不知道怎么做。
这样不仅无异于别人成长,有的时候甚至会让别人有挫败感。
那且帮别人写代码的方式可扩展性很差。
即使code review会花掉十倍于你自己写的时间和精力,但他会让人明白代码应该怎么写。
从长远来看,这其实是在一定程度上复制了你的生产力,你不能什么都自己写。
尤其是开始带项目,带新人以后,每天review五个人的代码和写五个人的代码。
长期而言,哪个更合算呢?答案显然是前者写代码是一个学习过程,怎么做一个好的代码,审核人更是一个学习和成长的过程。
自己绕过一个坑不难,难的是看到别人那么走,远远的,你就能告诉他那里有个坑。
而他在你的指导下以后,也会帮忙指出别人的类似问题。
我在这一点上,最近感触尤为深刻,随着团队越来越大,新人也越来越多。
有一段时间,我每天工作的一半时间都在review别人的代码写评论,这样做的初期确实比较辛苦,不过效果也随之慢慢显现。
大部分时间,工程师们已经可以进行相互review代码了,于是我可以腾出很多时间来做别的工作,代码,质量也得到了保障。
提交代码的类型,在进行code review之前,要搞清楚提交的代码到底是干嘛的,然后进行针对性的审核。
我们一般把提交的代码分成四类,一、bug修复,一般公司都有独立的bug追踪和管理系统,每一个bug都有一个票据代码提交的PR,一般和票据是关联的。
二、代码优化,比如文件的移动和拆分,部分函数的重构等。
三、系统迁移,包括代码库拆分用、另一种语言、重学等四新系统和新功能。
新功能在实现之前都需要进行设计审核。
最终版本的设计文档会包括数据库的schema、 API的签名、代码的流程和模块等内容。
相关代码的提交,也就是PR,一般是和设计文档挂钩的。
了解了提交代码的作用,审核就会更有针对性和效率,也更容易从作者的角度阅读代码。
最后说一下code review的注意事项,从代码提交者的角度,在代码审核中需要注意哪些问题呢?第一,为什么要进行PR?原因一定要在提交的时候写的非常清楚,才能帮助审核者理解这个改动是不是合理。
上面说的四种提交代码的类型具体是哪一种?应该写到PR的小结中写的越详细越好,这在以后需要进行回溯或追踪系统变化时也是很有意义的。
如果改动的是前端代码,最好贴一个改动前和改动后的截屏或改动,效果一目了然。
第二,除非是极其明显的单词拼写问题,尽量不要引入,不是这个PR目的的改动PR要尽可能保持目标的单一性。
每次遇到有人把一些代码结构的优化合并到功能相关的改动时,我就有一种肝火上升的感觉。
这种行为不仅会增加审核者的困难,降低效率,还会掩盖一些简单的错误。
并且如果因为功能的修改导致线上出了问题,一般需要退回到之前的版本,也就是反转PR.这时候针对优化相关的改动,也就必须被反转,总之是弊远远大于利。
第三,找谁审核。
有时候代码还会和其他项目组的代码相关,需要找该组的成员审核。
这时具体找谁呢?一般有两个机制来解决这个问题。
一是在github里at一个组,比如payment组、risk组等,这些组会通知组里所有的人相关的人看到了就回去审核。
二是有一些组的代码,不希望其他组的人在自己不知道的情况下进行改动,就会设置规则。
如果有人动了,这些代码也会通知到整个组,最后也是最重要的。
一定要确保所有的改动都是测试过的,无一例外从代码审核者的角度又需要注意哪些问题呢?审核的力度要多细,是不是每次审核都要花很多时间?当然如果时间足够,自然是看的越细越好。
如果特别忙的时候可以做一些筛选。
比如你可以看一下算法或者编程思路,然后加一个评论算法部分看来没有问题,也可以只看你关心的部分,然后加评论支付部分没问题,或者API部分没问题,还可以在艾特一些你觉得可以对其他部分加评论的人。
另外,如果是新人的代码,尽可能在代码、风格、性能等方面都加以审核。
如果是一个老员工,这些方面可以给予更多信任。
具体哪些方面需要审核呢?总结一下。
一、代码格式方面,许多公司都有编程语言风格指南,这是大家的约定俗成,避免公司的代码风格不一致,也避免了一些要不要把b括号另起一行的无谓争论。
老员工除非不小心,通常大家都不会弄错,新员工在这方面不太熟悉,就有可能出问题。
这一类问题是比较容易指出的。
二、代码可读性方面,比如函数不要太长太长就进行拆分,所有的变量名要能说明它的用意和类型,不要有嵌套太多层的条件,语句或者循环语句,不要有一个太长的布尔类型判断语句。
如果一个函数别人需要看你的长篇注释才能明白,那这个函数就一定有重构的空间。
另外,如果不可避免有一些注释,则一定要保证注释准确,且与代码完全一致。
三、业务边界和逻辑思角问题,你可以帮助代码。
作者想想他有没有漏掉任何业务边界和逻辑死角问题。
很多时候,这是业务逻辑相关的,尤其需要资深一点的工程师帮助指出需要处理的所有情况。
四、错误处理,这是最常见的问题,也是代码审核最容易帮别人指出来的问题。
我在文稿中举出了一个例子,一段简单到不能再简单的代码,就至少有三个潜在的问题。
这些都是需要审核者注意的。
五、确保测试用例覆盖到了所有的功能路径。
严格来说,每段代码都应该有测试用例。
如果开发者能够预见到其他人的代码改动,会引发自己的代码问题,一定要增加额外的测试用例,防止这种情况的发生。
六、代码质量和规范遵循公司制定的编程规范。
比如,如果有重复的代码段,就应该提取出来。
公用不要在代码里随意设常数,所有的常数都应该文件顶部统一定义,哪些变量应该是私有的,哪些应该是公有的等等。
七、代码架构包括代码文件的组织方式。
函数是不是抽象到live或者helper文件里,是不是应该使用继承类,是不是和整个代码库的风格一致。
Api的定义是不是restful等等公司层面的支持。
你从公司层面应该有哪些措施帮助员工有效的进行代码审核呢?一、统一的代码提交和审核流程与工具,并确保大家使用同样的工具,遵循同样的流程。
二、鼓励员工帮助别人审核代码,甚至可以做到绩效评估中。
三、制定统一的编程规范和代码风格,尤其是在有争议的地方,这样可以解决一些。
因为程序员偏好带来的矛盾,代码审核和编程一样,都是日常工作,不要情绪化。
我曾经做过一件事,一个外祖同事的代码,别人已经认可合并了,可是我觉得有问题,于是反转了流程,在评论里写了原因和建议的改法,当时心里还觉得会不会得罪人。
可是后来发现同事反而很客气的接受并道谢了。
另外,评论里经常会出现不同意见,其实是两个人在编程习惯和约定俗成上没有达成共识。
如果在不违背公司风格指南的情况下,没必要一定让对方和你有一样的改惯。
如果你真的觉得这样做更好,可以委婉的提出来。
比如我个人是更偏向于a类型的编程风格,不过这不是一个硬性规定,或者嗯改成a类型的编程风格,如何,不过这不是强制的。
今天,我和你讨论了code review主要内容包括code review中的重要概念代码合并的规则,帮助别人评审代码,而不是写代码提交代码的类型和做code review时的注意事项。
事实上,工程师们在编程的时候很难确保万无一失,多几双眼睛一起帮你看一遍,就可以很大程度的减少代码里的错误。
另外,互相审核的过程中,还能从彼此那里学到很多编程的小技巧和好习惯。
仔细想来,很多java和ruby的特别好用的技巧,都是我在做代码审核的过程。
中学到的。
你的团队有没有做code review呢?有好的经验可以在留言里告诉我,感谢您的收听,我们下期再见。