Code Review 代码审核最佳实践

Code Review,中文叫代码审查,指的是完成了部分功能的代码开发之后,在代码真正合并到仓库主分支之前,邀请同事帮你进行代码的审核和检查,检查代码的质量、规范、设计等等方面的过程。

代码审查的好处

知识共享

进行代码审查的好处很多,其中一个就是可以让团队的整体水平快速得到提升。一个团队中有资深的工程师也有刚毕业的人,水平参差不齐。对于刚进入团队的人来说,不熟悉很多团队的规范,那么就可以通过代码审核的过程进行指导和分享。

如果系统之间非常复杂,互相之间的依赖调用很多,一次代码的修改可能会影响其他系统的运行,这时候,如果有比较熟悉这个系统的人的提醒和指导就可以及时发现问题,新人可以在这个过程中学到系统设计的原则,如何编写可读性和复用性强的代码,学习到系统工程最佳实践等等。团队的水平因此而得以提升。

对于团队中的资深人士来说,他们可能觉得帮助新人会浪费自己的时间,其实不然。当整个团队更加优秀,水平更高,就可以帮忙分担更多重要的任务,而自己也可以抽身去解决更难的问题或者进行自我提升。

团队的沟通能力越强、设计能力越好其实也是节省了自己的时间。谁不希望可以高效地和别人合作完成任务呢?与其低水平地交流合作,事情还迟迟完成不了,不如投入一些时间提高团队的战斗力。

更好的代码质量

许多公司团队不注重代码质量,认为工作完成了,功能上线了就可以了。其实大家没有看到软件的生命周期中,成本最重的一个环节就是持续的维护和更新迭代。

代码质量差,是将来欠下的技术债。代码随心所欲地写,每个人有每个人的喜好,没有任何规范,也不注重可读性。虽然系统表面上解决了当下的问题,但是当团队成员流动,新人接手项目后,发现读不懂,代码如“屎”山,项目推翻重做,之前的投入付诸东流。这个用人成本是否有考虑过?

举个简单的例子,就像建造一个楼房,为了赶工期不注重工程质量,房子外形是符合交付标准了,但是房子内部的东西偷工减料,楼房的使用寿命没过多久就变成危楼,无法使用了,造成了大量的人力和资源的浪费。

开源软件对代码的审核非常严格,这也是为什么开源软件的质量那么高以及能够经久不衰。开源项目的委员会会非常严格地审核每一行代码。良好的设计和优秀的代码质量才得以让开源软件保持稳定的迭代,以及持久的生命力。

代码审查的目的

代码审查有诸多的好处,而它的目的就是要引入团队协作规范,让所有贡献代码的人都能够按照一定的流程和约定来完成开发任务。

许多公司的代码审查流于形式,只是走个过场,虽然需要别人同意才能够把代码合并进主分支,但是没有人对代码进行严格的考核,代码风格也没有统一的约束。

有些人对代码审查比较反感,认为是一种拖慢进度的强制流程。然而据我观察,对于那些能够真正运用好代码审查的公司,这对员工来说是一种福音,更像是大家共同建设的一种工程师文化。因为对于个人而言,可以快速地学习和提升,对于公司而言,软件的质量得到了保证,是一种双赢的举措。

代码审查的心态

良好的代码审查,少不了人和人之间建立起的良好氛围,如谦逊,友善,互信互助和成长型心态。

作为审核别人代码的人,需要就事论事,评论的语气要尊重他人,客观公正,带着帮助别人的心态去完成审核。不要带有主观色彩,也不要有针对个人的行为。

作为提交代码的人,不要害怕被别人发现自己的短板,不要对自己做的不好的地方遮遮掩掩。带着成长型的心态,发现问题就去解决,不会的东西就去学,虚心请教和学习,收起玻璃心,一切以成长为目的。一旦团队建立起良好的学习氛围,代码审核的机制会朝着正循环滚动。

代码审查的经验技巧

讲述了以上诸多的背景,来到正题,在国外大厂里,code review到底是怎么做的呢?

清晰的开发指导

  • 首先将团队要求、开发经验整理成一份清晰的开发指引,让新人通过阅读文档就能够知道应该怎么做,让团队成员能够步伐一致
  • 比如写单元测试的要求,代码风格如何统一,命名规范,异常信息如何写等等
  • 优秀示例: [1].Contributing to Apache Spark

自我检查

  • 提交代码前,先自己对照开发指导进行检查,避免PR(合并请求)被反复打回,减少不必要的时间浪费
  • 自己检查和发现一些低级的问题,这也是对别人时间的尊重

PR要足够小

  • 尽量保证每次code review的代码量在100行左右。太长的代码变化会让查看的人看起来很费劲
  • 功能变化较大的改动,也可以尽量拆分成若干个子任务或者模块,并在PR中描述清楚分别在实现哪个模块
  • PR足够小,对于双方来说都是双赢,审核代码的人能够看得更快速,而提交代码的人,可以让代码更快地合并到主分支

详细的上下文描述

有些人提交code review,什么也不说,就一段代码给到审核的人,以为别人什么都知道,应该知道他要实现的功能和达到的目的。

其实作为提交者,应该站在对方的角度思考,假设对方对自己的实施过程一点都不了解,应该怎么提供更加详细的信息。

  • 提供尽可能详细的前因后果。为什么要做这次改动,实现的目的是什么,是否有设计文档的链接,是否还有额外的参考文档,所有的信息都可以整理清楚写到描述框中
  • 如果你的代码只是实现了部分的功能,也要具体说明实现了哪一部分
  • 还可以在讨论区指出,自己为什么这样实现。虽然代码说明了一切,但是一些额外的描述,可以让对方更加容易理解,降低沟通的成本
  • 如果自己对某些代码存有疑虑,也可以指出并讨论,说明这不是最终的方案,还有待斟酌等

以上的目的,都是为了让对方更好理解自己的目的,详细的文档也为了以后再翻阅提供便利。

快速回应评论,关闭已解决的问题

  • 一旦有人评论了你的代码,应该尽快回复并解决,拖得越久,代码审核的成本越高
    • 一方面,在你审核的过程中,会有新的代码合并到主分支,会导致你需要不断地同步自己的代码,解决代码冲突,造成时间的浪费
    • 另一方面,时间长了,又要去回忆当时的一些讨论,会导致代码迟迟无法合并
  • 已经解决的问题,可以点击”Resolve“,让整个review的过程更加清晰,也知道自己的代码进度如何

找到正确的人帮你审核

  • 在一个大型的软件中,模块非常多,有些人特别熟悉某些模块,应该找到对应的负责人来协助你审核代码

如何审核别人的代码

系统设计

从系统和站在组织的角度,问以下问题:

  • 从系统的角度,这份代码的改动会引起兼容性的问题吗?它能够很好的跟其他系统整合吗?
  • 该代码的改动会不会引起系统故障或者影响其他依赖的系统的使用?
  • 现在是改变这个功能的恰当时机吗?整体的代码是否符合逻辑?

功能性

从功能性的角度考察:

  • 代码是否能够实现开发者想要的目标?
  • 这份代码是否包含测试?这些测试覆盖的场景是否足够多?是否考虑到了一些边界情况?并发的情况下性能怎么样,会不会产生死锁等问题?
  • 把自己想象成一个用户的角度,功能是否满足,是否考虑全面,是否存在一些bug?

代码风格和命名规范

  • 这些代码的命名是否规范?如果让一个不太熟悉的人来看这份代码,是否能够通过命名就了解其含义?
  • 代码有没有遵守约定的代码风格?比如 [2].Google Java style guides
  • 提交的代码如果想要做一些代码格式化的调整,如缩进和换行等等,需要放到另外的PR中进行,避免跟功能代码混在一起,给阅读代码的人造成不便。

测试

  • 如果代码中没有包含测试,要求开发者再附上完善的单元测试。原则上,每次代码改动都应该有其对应的单元测试。
  • 确保测试是正确的,合理且有意义的以及能够尽可能覆盖足够多的情况。

文档

  • 检查是否需要更新相应的文档。比如一次大的修改,需要记录一些release note或者README。或者一些必要的注释放到代码中,以备后续查阅。
  • 删除掉不必要的注释以及已弃用的代码等。

以上只是摘录了部分比较主要的review流程,如果想要了解更多的细节,可以到 [3].Google eng-practices guideline 查阅。

Code Review 黑话

Code Review过程中有一些常见的缩写/术语,列出来以备不时之需~

  • LGTM: Looks good to me. 这句话通常代表代码可以合并了。
  • PR: Pull request「合并请求」,有些地方也叫 Merge Request(MR).
  • NIT: nitpick. 意思是有点吹毛求疵,不是非常重要,常指一些代码风格的问题。
  • WIP: Work In Progress. 意思是代码还没有完全好,还在进行中,但是可以先整体看一下。
  • TL;DR: Too Long; Didn't Read 「太长懒得看」,README 文档常见
  • a.k.a.: also known as 「又称作」,比如 GitHub (a.k.a. GayHub) 。聊天消息里更常见到。
  • RFC: Request for comments 「请求意见稿」

Reference

[1].Contributing to Apache Spark

[2].Google Java style guides

[3].What to look for in code review

[4].Code Review Developer Guide by Google

[5].How We Do Code Review by Microsoft

[6].Code Review最佳实践 - 云+社区 - 腾讯云 (tencent.com)

[7]. 技术领域英文缩写和术语

[8]. Github 黑话大全

最后编辑于
©著作权归作者所有,转载或内容合作请联系作者
  • 序言:七十年代末,一起剥皮案震惊了整个滨河市,随后出现的几起案子,更是在滨河造成了极大的恐慌,老刑警刘岩,带你破解...
    沈念sama阅读 204,590评论 6 478
  • 序言:滨河连续发生了三起死亡事件,死亡现场离奇诡异,居然都是意外死亡,警方通过查阅死者的电脑和手机,发现死者居然都...
    沈念sama阅读 86,808评论 2 381
  • 文/潘晓璐 我一进店门,熙熙楼的掌柜王于贵愁眉苦脸地迎上来,“玉大人,你说我怎么就摊上这事。” “怎么了?”我有些...
    开封第一讲书人阅读 151,151评论 0 337
  • 文/不坏的土叔 我叫张陵,是天一观的道长。 经常有香客问我,道长,这世上最难降的妖魔是什么? 我笑而不...
    开封第一讲书人阅读 54,779评论 1 277
  • 正文 为了忘掉前任,我火速办了婚礼,结果婚礼上,老公的妹妹穿的比我还像新娘。我一直安慰自己,他们只是感情好,可当我...
    茶点故事阅读 63,773评论 5 367
  • 文/花漫 我一把揭开白布。 她就那样静静地躺着,像睡着了一般。 火红的嫁衣衬着肌肤如雪。 梳的纹丝不乱的头发上,一...
    开封第一讲书人阅读 48,656评论 1 281
  • 那天,我揣着相机与录音,去河边找鬼。 笑死,一个胖子当着我的面吹牛,可吹牛的内容都是我干的。 我是一名探鬼主播,决...
    沈念sama阅读 38,022评论 3 398
  • 文/苍兰香墨 我猛地睁开眼,长吁一口气:“原来是场噩梦啊……” “哼!你这毒妇竟也来了?” 一声冷哼从身侧响起,我...
    开封第一讲书人阅读 36,678评论 0 258
  • 序言:老挝万荣一对情侣失踪,失踪者是张志新(化名)和其女友刘颖,没想到半个月后,有当地人在树林里发现了一具尸体,经...
    沈念sama阅读 41,038评论 1 299
  • 正文 独居荒郊野岭守林人离奇死亡,尸身上长有42处带血的脓包…… 初始之章·张勋 以下内容为张勋视角 年9月15日...
    茶点故事阅读 35,659评论 2 321
  • 正文 我和宋清朗相恋三年,在试婚纱的时候发现自己被绿了。 大学时的朋友给我发了我未婚夫和他白月光在一起吃饭的照片。...
    茶点故事阅读 37,756评论 1 330
  • 序言:一个原本活蹦乱跳的男人离奇死亡,死状恐怖,灵堂内的尸体忽然破棺而出,到底是诈尸还是另有隐情,我是刑警宁泽,带...
    沈念sama阅读 33,411评论 4 321
  • 正文 年R本政府宣布,位于F岛的核电站,受9级特大地震影响,放射性物质发生泄漏。R本人自食恶果不足惜,却给世界环境...
    茶点故事阅读 39,005评论 3 307
  • 文/蒙蒙 一、第九天 我趴在偏房一处隐蔽的房顶上张望。 院中可真热闹,春花似锦、人声如沸。这庄子的主人今日做“春日...
    开封第一讲书人阅读 29,973评论 0 19
  • 文/苍兰香墨 我抬头看了看天上的太阳。三九已至,却和暖如春,着一层夹袄步出监牢的瞬间,已是汗流浃背。 一阵脚步声响...
    开封第一讲书人阅读 31,203评论 1 260
  • 我被黑心中介骗来泰国打工, 没想到刚下飞机就差点儿被人妖公主榨干…… 1. 我叫王不留,地道东北人。 一个月前我还...
    沈念sama阅读 45,053评论 2 350
  • 正文 我出身青楼,却偏偏与公主长得像,于是被迫代替她去往敌国和亲。 传闻我的和亲对象是个残疾皇子,可洞房花烛夜当晚...
    茶点故事阅读 42,495评论 2 343