写点什么

把代码评审做一百万次,学会了这些宝贵经验

  • 2020-01-31
  • 本文字数:2392 字

    阅读完需:约 8 分钟

把代码评审做一百万次,学会了这些宝贵经验

LinkedIn 的每个团队都使用同样的代码评审工具和流程,任何一个工程师都可以评审别人的代码,甚至可以为其他团队贡献代码。


LinkedIn 是全球知名的职业社交网站,在全球范围内有数亿用户。该公司的工程技术团队实力同样强劲,最出名的就是开源了 Kafka 等一系列流行技术。


2 年前,LinkedIn 的代码评审总量达到了里程碑式的一百万次。为此,LinkedIn 社交网络服务工具部门负责人分享了一路下来他总结的代码评审经验和教训。



阅读和评审代码是工程师每天都要做的事情。但是,这与正式的代码评审流程有所不同,正式的代码评审要求每一个代码变更在进入生产环境之前都要经过其他团队成员的评审。LinkedIn 从 2011 年开始将代码评审作为开发流程不可或缺的一部分。我们的代码评审目标是尽可能顺畅地在快速增长的团队中进行。好的代码评审(有意义且有用的评审注释)有助于提升整个公司的水平。在 LinkedIn,代码评审是质量保证和知识分享的一部分,并让整个工程文化朝着更好的方向发展。


在公司层面实现代码评审最大的一个好处是可以提高开发流程标准化水平。LinkedIn 的每个团队都使用同样的代码评审工具和流程,任何一个工程师都可以评审别人的代码,甚至可以为其他团队贡献代码。这也解决了诸如“我可以修复他们的 bug,但我不知道该怎么构建和提交”之类的问题,促进了各个技术部门之间的协作。


将代码评审作为开发流程不可或缺的一部分有助于形成一种健康的反馈文化:工程师乐于在工作的各个领域提供或接受反馈,而不仅仅是在代码层面,因为反馈已经成了日常工作的组成部分。在我们看来,提供和接受反馈都是很好的成长机会。事实上,高质量的代码评审是 LinkedIn 员工晋升的一个重要参考因素,因为它为工程师的技术能力提供了有力的证明。


在过去几年,我们总结出了一些最佳实践和技巧。下面以问答的形式提供了一些指南,帮助代码评审人员和被评审人员“榨干”代码评审的最后一滴油水。

我真的了解“为什么”代码要这么写吗?

为了促成最好的代码评审,每一个代码提交都应该包含设计概要,大致解释一下做出代码变更的动机是什么。如果只能从代码中推敲出代码变更背后的缘由,是很难实现高质量的代码评审的。代码提交者应该主动提供变更说明,把它们包含在提交日志里,这样也有助于提高代码文档的质量。

我提供了正向的反馈吗?

在一个到处都是聪明人的公司,获得干净的代码和整洁的测试覆盖率是理所应当的。正因为如此,代码评审一般都只关注代码中出现的问题。但问题是,大部分人需要从正向反馈中获得参与感和鼓舞,即使是工程师也不例外。如果评审人员发现代码中有值得称道的地方,就要把它们指出来,并给以正向的反馈。这样有助于提升团队士气,而且正向的反馈通常具有感染力。对于代码评审注释,任何正向的反馈都应该是明确的,如果认为代码写得好,就要说清楚好在哪里。

我的代码评审注释够清楚吗?

不管是正面的还是负面的反馈,每一个注释都应该能够清楚地表达评审人员的想法。面对太过简陋的评审注释,代码提交者根本无法领会评审人员的意图,尽管这些东西对于评审人员来说是显而易见的。如果有疑问,过度解释总比简陋的反馈要好得多,因为简陋的反馈会导致更多的问题,需要来回沟通,浪费时间。评审人员可以在注释里写上“减少重复”、“提升覆盖率”或者“让代码更容易测试”,等等。这样做除了可以让评审人员的注释更清晰明了,也有助于让团队达到更高的设计水准。

我是否认可代码提交者所付出的努力?

我们应该认可努力工作的人,不管其产出的成果是什么,这样有助于培养高士气的团队。有些代码质量不高,需要反复修改。对于这类情况,我们应该肯定代码提交者的努力工作,即使他们需要返工。最好的方式是在高质量的评审注释中加入适当的解释,认可好的想法(提交的代码中总有好的方面),并使用礼貌用语,比如“谢谢”。

这是个有用的评审注释吗?

通过问自己这个问题,可以很容易地知道一个评审注释是否是必要的。对于工程师来说,代码评审应该是有用的开发工具,而不是无用的负担。如果你觉得某个评审注释不是必需的,就把它删掉。比如,代码格式问题就不是一个必需的评审注释。代码风格和格式这类事情应该让自动化工具来做,工程师不应该关注这些问题。

“测试完毕”里的内容够详尽吗?

在 LinkedIn,每一个提交的代码变更都必须包含“测试完毕”部分。以 GitHub 为例,开发者在提交代码时会把“测试完毕”的内容放在 PR 描述里。“测试完毕”应该包含哪些信息?这取决于代码变更的重要程度和测试覆盖率。如果变更引入新的或者修改过的条件性复杂度,就有必要进行单元测试。在集成测试覆盖率不高的情况下,有些变更需要进行手动测试。对于这类情况,“测试完毕”部分应该包含测试场景描述和测试结果。如果代码变更会导致程序的输出发生变化,“测试完毕”部分需要包含新的测试结果。

我是不是太“学究”了?

有些代码评审注释里包含了太多信息,以至于一些不是很重要的建议把重要的问题——真正需要修复的问题——给“埋没”掉了。太痴迷于抠细节容易拖慢评审速度,还会挑起评审人员和代码提交者之间的矛盾。清晰的评审期望和积极正向的代码评审文化有助于避免出现拖沓冗长的评审。

总结

总的来说,正式的代码评审流程有助于提升代码质量,促进团队互相学习和知识共享。如果每个团队成员都能够意识到两件事情,他们的工作质量都将得到提升:既然我的代码需要经过评审,那就好好写。评审中反馈的每一个问题我都要解决,所以,为了以后不浪费时间,从一开始就要把代码写好。当代码评审成为一种习惯,整个团队都会很自觉地提供或接受反馈,这是成长和进步的关键。


我们从 LinkedIn 过去的一百万次代码评审中学到了很多,并希望从下一个一百万次评审中学到更多。在代码评审上投入越多,获得的代码评审效果就越好,提交的代码质量也越高,最终产品的质量也更高。高质量的代码评审是会“传染”的!

英文原文

LinkedIn’s Tips for Highly Effective Code Review


2020-01-31 09:3611407
用户头像
小智 让所有人认同的文字称不上表达

发布了 408 篇内容, 共 390.7 次阅读, 收获喜欢 1982 次。

关注

评论 1 条评论

发布
用户头像
注释 -> 评论
2020-08-28 12:57
回复
没有更多了
发现更多内容

架构1期第二周作业

FG佳

五种简单高效的拆分用户故事的方法

Bruce Talk

敏捷 Agile 用户故事 User Story Product Owner

架构师训练营第 1 期 - 第二周 - 作业提交

Todd-Lee

架构师 极客大学架构师训练营

八、给小白看的第一篇Python基础教程

刘润森

Python

[架构师训练营第1期]第二周命题作业

猫切切切切切

极客大学架构师训练营

「架构师训练营第1期」第二周作业

张国荣

极客大学架构师训练营

第二周学习总结

龙卷风

极客大学架构师训练营

高并发下如何缩短响应时间

架构师修行之路

微服务 高并发优化

九种查找算法

C语言与CPP编程

面试 算法 编程语言 C语言 编译器、程序语言、CPU

极客时间架构师培训 1 期 - 第 2 周作业

Kaven

架构师训练营 1 期 -- 第二周

小河

极客大学架构师训练营

C语言C++中assert的用法

C语言与CPP编程

程序员 编程语言 C语言

代码防御性编程的十条技巧

C语言与CPP编程

程序员 编程语言 C语言 编译器、程序语言、CPU

做好分库分表其实很难之一

架构师修行之路

微服务 分库分表

学生成绩管理系统案例

C语言与CPP编程

编程语言 C语言 编译器、程序语言、CPU

第二课框架设计课后作业

Geek_michael

数字与能源,交织成新基建的摩比斯环

脑极体

架构师训练营第一期 - week2 - 命题作业

谭明华

极客大学架构师训练营

架构师训练营 Week2 作业

lggl

极客大学架构师训练营 作业

四、学编程语言前,不了解Git,怎么入坑

刘润森

Python

前言、Python是真的火,还是炒得火?来看看它的前世和发展

刘润森

Python

一、搭建Python环境和安装Pycharm

刘润森

Python

二、搭建Jupyter Notebook环境

刘润森

Python

字符串操作的全面总结

C语言与CPP编程

编程语言 C语言 编译器、程序语言、CPU 字符串

架构训练营 -week2- 学习总结

于成龙

面向对象 架构训练营

三、新手Jupyter不会用,我十招教你盘她

刘润森

Python

五、开始Github和码云之旅,新手如何上路

刘润森

Python

架构师训练营第二周命题作业

成长者

极客大学架构师训练营

十、给小白看的第三篇Python基础教程

刘润森

Python

六、乘胜追击,将剩下的Git知识点搞定

刘润森

七、连Pycharm都不知道怎么用,学什么Python

刘润森

Python

把代码评审做一百万次,学会了这些宝贵经验_文化 & 方法_Szczepan Faber_InfoQ精选文章