google 如何进行代码审查02

原文
https://google.github.io/eng-practices/review/reviewer/looking-for.html
What to look for in a code review (程式审核过程中要看些什么?)
在考虑以下要点时，请务必连同The Standard of Code Review一起考虑。
Design (设计)
程式审核里最重要的一环是CL 的整体设计

CL 内多个程式片段的改动之间的交互作用是否合理? 改动是属于程式库(codebase) 还是函式库(library)? 此次改动是否能和系统其他部分很好地整合? 现在是加入该功能的适当时机吗?

Functionality (功能性)
CL是否真的有符合开发者意图?开发者的意图对于使用者们是好的吗?这里使用者通常系指終端使用者 (受到改動而影響)与開發者(未來會用到該程式的人)双方。

在大多数情况，我们会期望开发者彻底测试他们的CL，并期望在接受审核时它能够正常运作。但身为审核者你仍必须去思考可能的「极端案例(edge cases)」、找出「并发问题(concurrency problem)」、 尝试「作为一个使用者去思考」，并且确保「在单纯阅读程式码时没有明显的程式缺陷(bug)」。

愿意的话你也可以去亲自去验证CL - 如果当改动会带来直面使用者的影响(user-facing impact) 时，比方说UI 改动，验证CL 的变化便会是审核者在整个过程中最重要的事情。单纯阅读程式码有时很难体会它会如何影响使用者，面对如此的改动如果觉得不方便自己测试的话，可以要求开发者展示(demo) 此次改动的功能性。

另外，审核功能性时格外重要的一点是，要注意CL 中是否有「平行程式设计(parallel programming)」的存在，并是否在理论上会造成deadlocks 或race conditions。这类的问题较难透过执行程式来查觉，通常会需要有人(开发者跟审核者) 一起仔细思考，确保该问题不会被引入程式库内。

Note: 这也是不使用可能会造成deadlocks 或race conditions 的并发模型(concurrency model) 很好的理由- 它可能会让程式审核或理解上造成困难。

Complexity (复杂性)
一个CL 是否复杂到超过原本所必须的? 针对任何层级的CL 请务必确认这几点- 单行程式是否过于复杂? 函式是否过于复杂? Class 是否过于复杂?

「复杂」通常代表着该程式很难快速被阅读程式的人所理解

也意味着很有可能其他开发者在呼叫或修改这段程式时引入缺陷

其中就是一种复杂性便是「過度設計 (Over-engineering)」，开发者会让程式过度通用化(more generic)超过它原本所需的，或是新增现在系统不需要用到的功能[1]。审核者对于必须非常警惕过度设计的发生。

鼓励开发者解决他们現在需要解决的问题，而非那些猜测未来可能会需要被解决的问题

可以在真正面临到那些未来的问题时才解决它们，因届时你才能更清晰看到问题的样貌(actual shape) 与需求。

[1] 个人认为和Donald Knuth 说得「过早最佳化是万恶的根源(Premature optimization is the root of all evil)」有类似意思。自己在开发过程中也常常落入相同窠臼，认为未来可能会需要所以现在先写起来放。但更常见的状况是，当实际面临问题时，状况可能和原先大相径庭导致需要重写。
Tests (测试)
请将要求单元、整合、端到端测试视为要求CL所做的适当变更。一般CL内除production code以外，测试也应该要被加入其中。除非该CL是为了处理某个紧急事件而存在。

另外，也要确保测试是「正确(correct)」、「合理(sensible)」、「有用的(useful)」。测试并非来测试他们自己本身，一般也极少为了测试写测试(write tests for tests)，因此我们必须确保测试是有效的。

当程式真的出问题时，测试是否真的会失败? 如果被测试的程式发生改动时，测试是否会产生误报(false positives)? 每个测试是否有做出简单而有用的断言(assertions)?不同测试方法之间的测试是否适当分开?

请记住，测试也是必须维护的程式。不要因为它们不包含在main binary 内，

而接受测试程式中包含不必要的复杂性。

Naming (命名)
开发者是否为每个东西挑选了恰当的名字? [1]

一个好的命名意谓着

长到足以完整表达该东西的作用或做什么

同时不要长的难以阅读。

[1]推荐参考Clean code 101 — Meaningful names and functions
Comments (注释)
开发者是否用可理解的英文留下清晰的注释? 这些注释是否真的必要?

通常注释在解释为什么(explain why)这段程式存在(code exists)时相当有用，而不应该去解释某段程式正在做什么(not be explaining what some code is doing)。如果程式码本身不能清楚解释自己的话，意味着它需要更加简化(made simpler)。当然也有例外状况，比如解释正規表達式[1]或複雜的演算法正在做什么的注释往往相当有用，但对于大部分注释来说是用来说明那些不包含在程式本身的资讯，比方说为什么决定这样做背后的理由[ 2]。

查看该CL 之前的注释也很有帮助。或许有个TODO 项目现在可以移除、一个评论建议为什么不要进行这种改变等等。

要注意的是，注释与class、模组(module)、函式(function) 的文件不同。后三者要能够表达一段程式的目的、如何使用它、以及它在使用时的行为。

[1]请见先前关于Cloudflare全球大断线的文章
[2]常见的状况是基于商業上決定，而非程式面因素
Style (风格)
Google对于主要语言皆有提供风格指南，甚至包括大多数的冷门语言，因此请确保CL遵循适当的指南上的说明。

如果你想改进CL中某些不包含在风格指南中的风格要点时，请在评论前加上Nit:让开发人员知道这是你认为可以改善程式的小问题且并非强制性的。但记住，不要仅根据个人风格偏好阻止提交CL。

开发者不應在 CL 內同時包含主要風格的改動與其他程式的更改[1]，这样会导致难以看出CL到底做出什么改动。同时也会让合并(merge)和回滚(rollback)更为复杂，并产生其他问题。例如，如果作者想要重新格式化(reformat)整个档案的话，让他们将重新格式拆成一个CL，然后再发送另一个包含功能修改的CL [2]。

[1] 比方说CL 内包含上百个更改缩排的文件，中间额外穿插程式的异动
[2]通常revert时会希望commit越小越好，如此能更易于回滚，请参考Atomic Commits with Git
Documentation (文件)
如果CL 更改用户如何「构建(build)」、「测试」、「交互(interact)」、「发布」程式的方法时，请检查是否也同时更新相关文档， 包括READMEs，g3doc 页面和其他生成(generated ) 的参考文件。如果CL 删除或弃用(deprecate) 程式，请考虑是否应该删除对应文档，如果缺少文档时请询问。

Every Line (每一行程式)
仔细查看你被分配审核的CL 的每一行程式

有些东西，比如资料文件(data files)、生成的程式(generated code)、大型资料结构(large data structures)，你可以稍微扫过。 千萬不要在掃過開發者寫的 class、函式、程式區塊 (block of code) 時，並假設其內部的內容是沒問題的。 很显然的某些程式需要比其他程式更仔细的审查(scrutiny) -这是必须由你做出的判断(judgment call)。但至少你应该确定你理解所有程式在做些什么。

如果阅读程式过于复杂并且减慢审核速度时，那么你再继续审核前要让开发者知道这件事，并等待他们为程式做出解释、澄清(clarify)。在Google 我们聘请许多优秀的软体工程师，而你也是其中的一员。如果连你也无法理解程式的话，很可能其他人也不会。因此，你要求开发者去澄清此程式时，同时也在帮助未来的开发人员理解这些它们。

如果你能够理解程式但觉得没有资格进行某部份的审核，请确保CL 审核者中有一个适合(合格) 的审核人来审核该部分。尤其是针对「安全性(security)」、 「并发性(concurrency)」、「可访问性(accessibility)」、「国际化(internationalization)」等复杂问题时。

Context (前后文)
在充足的前后文(broad context) 下查看CL 通常很有帮助。

一般来说，程式码审查工具只会显示修改部分周围的几行程式而已。但有时你必须查看整个文件以确保改动是否合理。比方说，你可能只看到添加4 行新程式码，但实际上查看整个文件时会发现这4 行是被加在有50 行的函式里，此时便需要将它拆解为更小的函式。

以整个系统的角度出发来思考CL也是很有用的。该CL是否改善整体系统的程式品质，亦或是会让整个系统更加复杂?是否缺少测试? 千萬不要接受會降低整體系統程式品質運行狀況的 CL。因为大多数系统是由于许多小改动的积累而变得复杂，因此阻止新的改动引入复杂性(尽管很小)也非常重要。

Good Things (好事、优点)
当你在CL 里看到优点时记得告诉开发者，尤其是当他们用很棒的方式来解决你的评论时。

程式审查通常只关注在错误，但它们也应该同时应该为良好实践(good practices)提供鼓励与欣赏。这点在指导(mentoring)开发者时尤其重要: 比告訴告訴他們做錯了什麼，更好的是告訴他們做對什麼[1]。

[1] 个人认为并非不要指出错误，而是「多以鼓励来代替指出错误」，让其他开发者更有动力想将事情做好。其实透过简单的一句话让对方知道哪里做得很好，未来他们会将继续保持下去，并为其他开发者带来的正面影响
good-thing

对方注明每个commit 修改什么，可以帮助审核者快速进入状况

compliment
此时就不要吝于给出你的称赞吧!

Summary (总结)
在进行程式审核时，请务必确保

程式经过完善的设计
功能性对于使用者们是好的
对于任何UI 改动要合理且合眼(look good)
任何平行程式设计(parallel programming) 的实现是安全
程式不应该复杂超过原本所必须的
开发者不该实现一个现在不用而未来可能(might) 需要的功能
程式有适当的单元测试
测试经过完善的设计
开发者对于每样东西有使用清晰、明了的命名
注释要清楚且有用，并只用来解释「为什么存在(why)」而非「做什么(what)」
程式有适当的写下文件(一般在g3doc)
程式符合风格指南
确保你查看被要求审核的每一行程式码、确认上下文 (context)、确保你正在改善程式品質，并赞扬开发人员所做的好事与优点吧!