google 如何进行代码审查01

前言

贡献开源专案Fission时，不仅会被他人审核也常需要审核来自世界各地开发者的PR，从中体会到不少身为一个软体工程师、一个专案维护者该有的行为与谈吐。近期Google公布其内部关于「如何进行程式审核」的指导原则，快速浏览相关内容后真的是心有戚戚焉，要是能更早知道这些技巧就不会摸索这么长的时间。

认为这么棒的文章没有让更多人看到实在非常可惜，因此尝试自己翻译来分享给中文的开发者。若有文句不通畅、翻译错误、typo，还烦请多包涵与指教。(部分找不到适当中文翻译之专业名词会以原文呈现)

原文

https://google.github.io/eng-practices/review/reviewer/standard.html

The Standard of Code Review (程式审核准则)
Code review 主要目的在于确保Google 的程式库(code base) 整体健康度随着时间推移能有所改善。而所有的审核工具与流程皆是因应这个目的而生。为达到前述目的，势必需要做出一连串的权衡与取舍(trade-offs)。

首先，开发者必须能够在任务上取得进展，若不对程式库进行任何提交的话，那程式库永远不会有任何改善。此外若审核者(reviewer) 让任何提交变得难以进入程式库的话，则开发者会把「做出改善」这件事本身视为没有益处，并且未来不再做出任何改善[1]。

另一方面，审核者必须确保每个CL (changelist) 的品质，且不会让程式库健康状况随着时间推移而下降。尽管每个小改动只会造成品质微幅下降，但随着时间拉长逐渐累积后，最后终将变成压垮整体品质的那根稻草。而这通常是非常棘手的事情，尤其是当团队面临时间压力时，会让他们认为必须抄小径才能够达成原订目标。

再者，审核者对他们正在审核的程式有所有权与责任(ownership and responsibility)，确保程式库能够维持一致、易于维护，以及其他写于What to look for in a code review的准则。

从中我们得到一个作为我们预期在程式审核过程中出现的原则(标准):

一般而言，只要更动(CL) 有助于提升程式库整体品质的话，审核者就应该批准它

尽管它并非完美

这点也是位于其他程式审核原则之上最高级的原则。

当然在某些状况下这个原则也是会受到限制。比方说，某个CL 涵括审核者不希望出现在系统的功能时，此时尽管CL 的改动设计非常良好，审核者依然可以拒绝批准它。

主要重点在于「並沒有所謂完美的程式，只有更佳的程式」

审核者不该要求开发者在批准程式前仔细清理、擦拭CL每个角落[2]。相反的，審核者應該在 CL 的完善與其所帶來更動的重要性間做出權衡。与其追求「完美」，身为审核者更应该追寻「持續性的進步(continuous improvement)」。只要CL对整体系统能改善「维护性(maintainability)」、「可读性(readability)」、「理解性(understandability)」的话，就不应该因为它不完美而延迟批准数天甚至数周。

审核者也要习惯于针对CL可以改善的地方留下审核评论(review comment)。如果指出的改正不是非常重要的话，可以在前面加上Nit:让开发者知道该改正是可以选择性被忽略的[3]。

Note:这份文件并非尝试去证明合并CL绝对会造成程式库品质下降的问题。唯一会发生这种事的应该是在紧急情况 [4]。

[1]原文使用disincentivized，找到最接近的解释是「移除做某些事的益处，导致其他人不想再做」。
[2] 将程式码清理到完全符合审核者标准
[3] Nit全称为Nitpick，意指吹毛求疵、鸡蛋里挑骨头。自己本身会使用nitpick，比如以下范例。
nitpick
利用nitpick 告知不在风格指南上的小问题

[4] 因紧急状况而合并不完全符合品质要求的CL
Mentoring (指导)
程式审核在教导开发者新的东西如程式语言、框架、通用软体开发原则时扮演非常重要的角色。适时的留下评论来协助他人学会新东西永远是件好事， 分享知識亦会让整体程式品质随着时间推移下得到提升。

记得一点，如果你的评论是纯粹教育性质的话(意即并非严重到必须去符合文件所描述的标准)可以在评论前加上Nit:或是用其他方式告知这并非强制性需要在该CL解决的问题[1]。

[1]除加上nitpick外，若评论牵涉改动并非很重要时且不急着解决时，自己会明确告知开发者说let’s resolve this in another PR，避免PR被block住。
Principles (原则)
以技术事实与资料(数据) 为根本，否决意见与个人偏好

关于程式风格的问题，风格指南是绝对的[1]。任何没有列在其上的风格要点(如空格等等)则关乎每个人的个人喜好。一般来说，风格要和已经存在的程式保持一致。如果原先不存在任何风格的话，则接受CL作者的。

任何有关「软体设计(software design)」的层面，从来都不单纯是风格或个人喜好的问题。软体设计必须要建立在基础原则之上，并且在众多原则间做出权衡，而不该仅是个人意见。有时候若有数个不同的可行方案，只要作者能够展示(根据数据或完善的工程原则) 这些方案彼此是相等的(equally valid)，审核者则应该接受作者的选择。否则，设计方案的选择则应该追寻软体设计的标准原则来进行。

如果没有其他规则适用的话，则审核者可以要求作者和现有的程式库保持一致风格，只要这样不会让整体系统程式的品质恶化的话。

[1] 此指Google 风格指南
Resolving Conflicts (解决冲突)
在任何冲突发生时，第一步永远是开发者与审核者双方根据此份及其他在「CL作者指南」以及「审核者指南」的文件来尝试达成共识。

如果达成共识变得尤其困难时，双方来个面对面会议(face-to-face meeting) 或视讯会议(VC) 或许会有所帮助，而不要仅依靠透过评论来试着解决冲突[1]。(如果选择采用face-to-face meeting & VC，请确保将最后结论记录在CL 的评论里，以便后人查阅。)

如果上述做法仍无法解决冲突的话，最常见的作法是拉高处理问题的层级。从原先的两人讨论变成团队讨论、团队领导(TL) 来权衡(weight in)、 让程式维护者(maintainer) 来决定，又或者由工程经理(Eng Manager) 介入处理。

千万不要让改动因为作者跟审核者间无法达成共识而停留在那边

[1] 纯文字有时效率缓慢且容易造成误解对方的情绪，透过对谈方能快速理解与同步彼此的想法。这种状况对于远端工作的团队尤其重要，自己遇到类似情形会直接开视讯会议跟对方讨论，确保彼此想法一致(on the same page)。