译者前言
原文是 Google 工程师 Michael Lynch 的个人博客文章：
- https://mtlynch.io/human-code-reviews-1/
- https://mtlynch.io/human-code-reviews-2/
读了之后深有感触，目前国内大多数公司对于 Code review 的重视程度还远远不够，大多数人都把它视为一件麻烦事。即使在有 Code review 流程的团队，也缺乏相关经验，而且目前中文技术圈关于 Code review 的文章真的太少了，所以在这里翻译这篇个人认为很不错的文章给大家看。
正文
最近，我读了很多关于 code review 最佳实践的文章。但我发现大多数文章都着重于教你如何找到代码中的 bug，而几乎完全忽略了 code review 的其他部分。比如你沟通的方式是否足够建设性及专业呢？无所谓！只要找到所有 bug，剩下的就自生自灭去吧。
所以我获得了一个启示：如果这些跟代码相关，为什么不能以一种更浪漫的形式呢？于是，我想把我的新书介绍给开发者们，以帮助他们以及他们热爱的生活。
我这本革命性的书将会教你一些实用的技巧，以帮助你最大限度地找到你同伴身上的所有缺点。但它不会包括以下内容：
- 如何与你的同伴沟通，让你们相互理解、产生共鸣
- 如何帮助你的同伴找到他们的不足之处
这本书适合你吗？我仿佛听到了你喊“不不不不！”
所以，为什么我们一定要用那种（没人性的）方式来讨论 code review 呢？
唯一的回答就是，我是在遥远的未来读这本书的，那个时代所有的开发者都是机器人。在那个世界里，你的团队小伙伴很喜欢冰冷的、生硬的、无情的评论，因为它们都是机器人，阅读这些评论能温暖它们的机械之心。
但我想做一个大胆的假设，你现在就想改善你们团队的 code review，而你们的团队成员都是活生生的人类。我还想做一个更大胆的假设，那就是“与同事建立积极的关系”本身就是一个目的，而不是简单地调整某个变量，以减少 bug。在这种情况下，你的 code review 会发生怎样的变化呢？
我的这篇文章会介绍一些技巧，让 code review 不再仅仅是一种技术上的流程，更是一种社交性的流程。
什么是 Code Review？
“Code Review” 这个术语实际上包含了一个很大范围内的活动，从最简单的读同伴的代码，到 20 个人正襟危坐在会议室里一行一行地剖析代码，都可以称为 “Code Review”。在后文里，我用这个术语来指代一个正式且书面化的过程，但不是像内部 Code Review 会议那样重量级。
参与 review 的人有两种，一种是作者（author），也就是代码的编写者和 review 的发起者；另一种是评审者（reviewer），也就是阅读代码，并且决定代码是否可以合并进入团队代码库的人。评审者可以有多个人，但后文里我会假设你是唯一的评审者。
在 review 开始之前，作者必须创建一个变更列表（changelist）。它含有一系列的修改，而作者想要把这些修改合并进入代码库。
当作者把变更列表交给评审者之后，review 就开始了。review 是一种轮转的方式进行的，每一轮都是一次作者和评审者之间的往返：作者提交变更列表，评审者对这些变更作出反馈。每个 review 都会有一轮或多轮。
当评审者**同意（approve）**这些更改之后，code review 就结束了。这个时候一般会评论一句 “LGTM”，也就是 “looks good to me” 的缩写。
这有哪些难点？
设想一下，如果一名程序员给你递交了一份他们自认为很赞的变更列表，而你回复了一大堆问题，告诉他这份变更列表并不好，这里就很容易让人感到冒犯。
这就是为啥我从来不怀念 IT 行业的原因，程序员都是些很不讨人喜欢的人… 要是放在航天行业，这些高估自己能力的人坟头草都已经一米多高了. --- Philip Greenspun，ArsDigita 联合创始人
对于代码的作者来说，评论或者批评他们的代码，很容易被视为一种暗示，即他们不是一名称职的程序员。虽然 code review 是一个很好的机会来分享知识、做一些工程上的抉择，但如果 code review 被视为人身攻击，那么这些好处都不会发生。
就算上面这种情况不会发生，你也会遇到沟通的问题，把你脑子里的想法用文字准确地表述下来是很具有挑战性的，因为别人很容易产生误解。代码的作者听不到你的语音语调，也看不到你的肢体语言，所以清楚的写下你的反馈是一件很重要的事情。对于戒备心理很强的代码作者而言，一句无意的 “你忘了关掉 file handle”，可以被理解为，“你竟然忘了关闭 file handle？你这个蠢猪。”
技巧
让电脑做重复的事情
在会议和邮件的交替轰炸中，你能专注于代码的时间实际上是很稀有的，你的精神耐受力甚至更短。读团队小伙伴的代码需要大量的精力和高度的精神集中，所以不要把精力花在计算机可以代替我们做的事情上，特别是那些计算机做得更好的事情。
空格问题就是一个明显的例子。比较一下，评审者靠肉眼找到一处缩进错误，然后协助代码作者修正它，和直接使用一个代码格式化工具，哪一个耗费的时间更少？
| 靠评审者的人力 | 靠代码格式化工具 |
|---|---|
| 评审者靠肉眼找到缩进错误 | |
| 评审者写了一条评论来标注这个错误 | |
| 作者读这条评论 | 什！么！事！都！不！需！要！干！ |
| 作者修正缩进错误 | |
| 评审者再次检查，确认错误已修复 |
表格的右边之所以啥都不需要干，是因为作者已经使用了一个自动格式化工具，在每次保存代码时，都会自动执行。最坏的情况下，作者没检查代码就提交 review 了，持续集成系统也会报错，这样作者就可以在评审者发现之前就修复这个问题。
在你的 code review 中找到可以被自动化的地方，下面是一些常见的点：
| 任务 | 自动化解决方案 |
|---|---|
| 确认代码可以构建无误 | 持续集成系统，例如 Travis 或者 CircleCI |
| 确认代码可以通过自动化测试 | 持续集成系统，例如 Travis 或者 CircleCI |
| 确认代码的空格和缩进符合团队规范 | 代码格式化工具，例如 ClangFormat (C/C++ formatter) 或者 gofmt (Go formatter) |
| 找出无用的 imports 或者无用的变量 | 代码格式检查工具，例如 pyflakes (Python linter) 或者 JSLint (JavaScript linter) |
自动化可以让作为评审者的你做更多有意义的事情。当你可以不需要在意某大类问题（例如 imports 的顺序、源文件的命名约定），你就可以有更多的精力关注其他更有趣的问题，例如函数错误或者可读性差的问题。
自动化同样可以让代码的作者受益，它可以让作者快速地在几秒钟（而不是几小时）内找到一些粗心产生的低级错误。快速的错误反馈产生的修复成本也很低，因为代码作者的脑中依然有这段代码的上下文。另外，来自电脑的报错相比于来自评审者的纠错，从自尊心上讲，更容易让人接受。
你的团队应该立刻把这样一套自动化工具加入到 code review 的工作流中（比如 Git 的 pre-commit hook，还有 Github 的 webhook）。如果 review 需要评审者手工去做这些事情，那就丧失了很多益处。代码的作者总是会忘记遵守某些东西，以至于你必须重复地检查很多简单又低级的问题，而这些问题本应是自动化工具代替你做的。
用代码风格规范来解决代码风格的争议
关于代码风格的争吵在 code review 中是非常浪费时间的。一致的代码风格当然是非常重要的，但 code review 的时间并不该浪费在讨论圆括号该放在哪里。最好的做法是通过维护一份代码风格规范来避免这里的争吵。
一份优秀的代码风格规范，不仅仅定义了诸如命名规范、空格规范这些表面上的东西，同样也应该定义如何使用语言的某些特性。例如 JavaScript 和 Perl，它们具有函数式编程的特性 —— 也就是说它们提供了多种方式来完成同一件事情。代码规范里应该只定义一种正确的方式，这样的话才能让你的团队不会一半的人用某些语言特性，而另一半的人用完全不同的其它特性。
当你有了一份风格规范后，你就再也不需要把时间浪费在讨论谁的命名规范最好这种问题上了，只要按照规范来就可以。如果你的风格指南没有针对某个特殊问题作出规定，那么它在 review 过程中不应该被讨论。如果遇到规范中没有涵盖的问题，并且这个问题足够重要，那么可以与团队成员进行讨论，然后将讨论结果记录在代码风格规范中，这样你们以后就不用再讨论了。
选项一：使用一份已有的代码规范
在网上搜一搜，就能找到不少已有的代码规范，其中 Google Style Guide 是最广为人知的。当然，如果它不适合你的话，你也可以用别的规范。使用已有的规范，你可以直接获得收益，而不需要从零开始创建一份规范。
直接复用一份现成的规范，缺点在于，这些规范可能是为了原团队中某些特殊需求而优化的。比如 Google 的代码规范，对于新的语言特性十分保守，因为他们有一个巨大无比的代码库，这些代码可能会运行在很多地方，从家庭路由器，到最新款的 iPhone 上。如果你所在的是只有四个人和一款产品的小型初创公司，那么你可能会更喜欢使用最尖端的语言特性或者扩展。
选项二：渐进式地建立你自己的代码规范
如果不想直接使用现成的代码规范，当然可以自己创建一份。每当 code review 时产生了关于代码风格的争议，就把这个问题提给团队所有成员，来决定正式的标准。达成一致后，把这个标准写入你的代码规范中。
我一般喜欢把我团队的代码风格规范以 Markdown 的格式托管在版本控制软件之下（比如 Github pages）。这样，对规范的任何修改都会经过一个正式的 review 流程 —— 必须有某人明确地批准修改，团队中的任何人都有机会提出疑虑。用 Wiki 和 Google Docs 当然也是可以的。
选项三：混合式的方法
结合选项一和选项二，你可以用一份现成的代码规范，在它的基础上，建立你自己的代码格式规范。比如 Chromium C++ style guide 就是个很好的例子，它建立在 Google C++ style 的基础之上，但有它自己修改或添加的一些规则。
立刻开始 Review
code review 应该被视作高优先级的事情。你阅读代码并提供反馈意见时，花点时间无所谓，但 review 必须要立刻开始 —— 最好是在几分钟内。
Code Review 的接力赛
一旦团队成员提交给了你一份变更列表，这可能意味着在你的 review 完成之前，他们的工作会被阻塞住。虽然在理论上，版本控制系统可以让代码的作者切换到新的分支，然后把审核中的提交合并到新的分支，继续工作。但实际上，只有很少的人能高效率地做这些事情，因为这需要同时同步三个分支（译者注：master、review 中的分支、新分支）的变动。
当你立即开始 code review，你就创造了一个良性循环。你一轮 review 所需要花的时间，和变更列表的大小及复杂度成正相关。这就鼓励了代码的作者提交小范围的变更列表，这也让你的 review 变得更轻松愉悦，你的 review 也会更快，形成一个正向循环。
想象一下，你的小伙伴实现了一个新功能，需要改变 1000 行代码。 如果他们知道你可以在大约 2 小时内查看 200 行的更改列表，则可以将其功能分解为多个变更，每个约 200 行，于是你就可以在一两天内 review 完毕。 但是，如果你每个 review 都是拖了一天之后才开始 ，那么就需要花费几乎一周的时间才能做完 review。你的小伙伴当然不想就呆坐在那里一周，因此他们就会偏向于提交更大体积的 review，比如500-600行。 这些大的 review 要花的时间更多，而且会产生质量更差的反馈意见（因为你要在脑内记住 600 行的变化而不是 200 行）。
一轮 review 的最大周期应该限于一个工作日，如果你因为某些优先级更高的事情忙成狗了，那么请你告诉你的小伙伴，让他们把 review 的任务交给别人。如果每个月都有几次这样的情况发生，那就说明你的团队需要减速了，这样才能保证团队的开发不会失去控制（译者注：在中国就别想了）
自顶向下的方法
你在一轮 review 中提出的问题越多，代码作者感到的压力就会越大。具体数量的限制因人而异，但一般一轮 review 提出的问题应该限制在 20 - 50 个之内。
如果你担心评论太多，把代码原作者淹没在茫茫的问题之中，那么建议你在早期的 review 中先关注一些高层次的问题，例如重新设计类的接口，以及分解复杂函数等。直到这些问题得到解决，再去关注低层次的问题，比如变量命名，或者代码注释的清晰程度。
代码原作者关注高层次问题时，一些低层次的问题可能会被忽视。把这些低层次问题暂时延后到下一轮 review，就可以避免重复检查这些低级问题，也可以节省代码原作者的时间。这个小技巧可以让你在 review 过程中对应该关注的层面进行细分，帮助你和原作者以一种更加清晰、系统的方式处理更改列表。
多写代码示例
在理想的世界里，代码作者应该会很感谢他们收到的每一个 review，因为这是一个很好的学习机会，并且让他们纠正了错误。但在现实中，有诸多因素可能会导致作者负面地看待这些 review，并且反感你对他们代码的评论。也许他们正面临着压力，要在最后期限之前完成任务，所以除了即刻批准以外的任何事情都会被视为一种阻碍。 也许你们之间没有太多的合作经验，所以他们不相信你的反馈是好意的。
这有一个好方法，可以在 review 过程中舒缓作者的心情，那就是在 review 期间找机会送给他们礼物。所有开发者都喜欢接受的礼物是什么？那当然是，代码示例。
你可以通过写一些示例代码来减轻作者的负担，以展现出你作为评审者的慷慨大度。
比如说，你有一个同事并不是很熟悉 Python 的列表推导特性，他给你发了如下的代码：
urls = []
for path in paths:
url = 'https://'
url += domain
url += path
urls.append(url)
复制代码作为回复，一句 “我们能不能用列表推导来简化这儿的代码？” 可能会让他们感到恼怒，因为他们或许需要花 20 分钟来搜索他们之前从没用过的东西。
但如果收到的评论是像下面这样，他们应该会很高兴：
可以考虑这样简化代码：
urls = ['https://' + domain + path for path in paths] 复制代码
这个小技巧不仅限于单行代码。我会经常创建自己的分支来向原作者展示大量的概念，比如拆解一个大的函数，或者添加单元测试来覆盖额外的边界情况。
这个小技巧会让你得到明确的、无争议的改进。在上面的示例中，很少有人会反对将代码行数减少83％。相比之下，如果你写了个冗长的例子来展示你个人品味上觉得 “更好” 的示例（例如，代码风格），这会使你看起来更一意孤行，而不是开明大方。
当然示例代码也不能写得太多了，如果你为原作者写了几乎覆盖整个变更列表的示例代码，那就表示你不认为他们有能力编写自己的代码。
不要说“你”
这听起来有些奇怪，但请相信我说的：在 code review 中，不要使用 “你” 这个词。
你在 review 中所做的评论应该是基于 “什么使得代码更好”，而不是 “谁提出了这个想法”。你的小伙伴在他们的变更列表上花费了大量的精力，他们当然会为他们所做的工作感到自豪，所以当收到批评时，自然会产生戒备心理。
你应该用一种最不会产生戒备心理的方式来评论代码。要记住你批评的是代码，而不是代码的作者。当代码的作者在评论里看到 “你” 这个词的时候，会把注意力从代码上转移到他们自己身上。这会加剧他们抗拒你的评论的可能性。
比如说下面这个无恶意的评论：
你把 'successfully' 拼错了
作者可能会把它读成两种意思：
- 含义一：嘿，好兄弟！你把 'successfully' 拼错了，但我认为你这么聪明，这应该只是一处小粗心吧！
- 含义二：你把 'successfully' 拼错了！白痴！
相比之下，如果你的评论里没有提及 “你” 这个词：
sucessfully -> successfully
这条评论就只是简单地纠正了拼写错误，没有包含任何对于作者的评价。
幸运的是，在评论中去掉 “你” 这个词非常容易。
选项一：用 “我们” 代替 “你”
你能不能把这个变量名写得更清晰一点？比如
seconds_remaining？
修改之后：
我们能不能把这个变量名写得更清晰一点？比如
seconds_remaining？
“我们” 这个词强调了团队对于代码的责任。代码的作者可能未来会去别的公司，你也可能会，但这里的代码会被它所属的团队一直维护着。当然，用 “我们” 这个词听起来有些愚蠢，因为这显然是你作为一名评审者，要求作者做的事情，但愚蠢总比指责更好。
选项二：移除句子的主语
避免使用 “你” 的另一个方法是移除句子的主语：
建议使用更清晰的变量名，比如
seconds_remaining。
当然被动语态也是可以的。虽然我在写技术文章时尽量避免使用被动语态，但它在 “你” 这个词的问题上确实有用处：
这个变量应该使用更清晰的名字，比如
seconds_remaining。
还有一种方法是把它化为一个问题：
要把这个变量名换成更清晰的吗？比如
seconds_remaining？
使用请求的语气，而不是命令
Code review 比日常的交流需要更多的精力，因为很容易把讨论变成个人观点的碰撞。你总是期望评审者能在评论中保持礼貌，但奇怪的是，他们总是和期望相反。日常工作中，大多数人都不会对同事说：“把订书机拿给我，然后给我倒一杯苏打水过来。” 但 review 过程中却经常看到类似这样的评论：“把这个 class 放到另一个文件里。”
这样的语句经常会让你的评论令人恼怒。你的评论应该使用请求或者建议的语气，而不是命令。
比较下面这两种语气的区别：
| 命令 | 请求 |
|---|---|
| 把 Foo class 移到一个单独的文件里 | 我们能不能把 Foo class 移到一个单独的文件里？ |
大多数人都喜欢完全掌控他们的工作，使用请求的语气可以让他们有自主的感觉。
另外，请求的语气也让作者更容易礼貌地推辞你的评论，或许他们是出于某些原因，考虑过后才写下的代码。如果你的语气是命令式的，那么作者的任何推辞和解释都会变成直接的不服从行为。如果你用的是请求或者提问的语气，那么作者可以简单地回复你。
比较下面这两种情况：
| 命令（对抗的语气） | 请求（相互合作的语气） |
|---|---|
| 评审者：把 Foo class 放到单独的文件里 | 评审者：我们能不能把 Foo class 放到单独的文件里？ |
| 作者：我拒绝，因为那样的话它就会远离 Bar class。客户端里几乎都是一起用这两个 class 的。 | 作者：可以啊，但如果这样的话，它就会远离 Bar class，客户端几乎总是一起使用这两个 class 的，你觉得该怎么做？ |
看，当你的语气变成请求式之后，是不是交流少了很多火气呢？
评论应该基于原则，而不是观点
当你写下一条评论，要记得同时写下要做什么以及为什么要做。说 “我们应该把这个 class 切分成两个” 是不太好的，更好的说法是 “现在这个 class 负责下载并且解析文件。根据单一职责原则，我们应该把它切分成两个 class，一个负责下载，一个负责解析。”
你的评论应该是基于原则的，这样才可以让 review 是建设性的。当你有一个明确的理由为什么要这样做时，比如 “我们应该把这个方法私有化，以减少公共方法的数量”，作者一般都不会简单地回复 “不，我更喜欢现在这样”。即使他们这样简单地回复了，这样的回复也会看起来很傻，因为你都已经说明理由了，而他们只是因为个人偏好而拒绝你的理由。
软件开发既是一门艺术，也是一门科学。有些时候，即使有了既定的原则，你也不能清楚地证明一段代码是错误，因为有时代码只是丑陋或不直观而已。在这些情况下，请详细解释一下为什么，并保持客观。比如如果你说 “我觉得这很难理解”，这就是一个客观的陈述。但如果你说 “这里写得乱七八糟”，这就是一种价值判断，是仁者见仁智者见智的。
另外，在提供支持性材料的时候，尽可能以链接的形式附在后面。团队的代码风格规范是最好的，当然你也可以发一个语言或库文档的链接，高赞数的 StackOverflow 答案也可以。但是要知道的是，离权威性文档越远，材料就越无力。
第二部分
如果你喜欢这篇文章，还可以去看它的第二部分，着重于介绍如何让 Code Review 顺利完成，而不是各种碰壁。第二篇文章会介绍以下技巧：
- 如何处理超大的 Code review
- 恰当地称赞对方
- 限制 Review 的范围
- 如何缓解僵局
扫一扫
5491
被折叠的 条评论
为什么被折叠?
到【灌水乐园】发言
