Google是如何做Code Review的

我和幾個(gè)小伙伴一起翻譯了Google前一段時(shí)間放出來的Google’s Engineering Practices documentation，翻譯后的github倉庫https://github.com/xindoo/eng-practices-cn，歡迎加star。目前只是翻譯完了，因?yàn)樽g者水平有限，還需要審校。另外后續(xù)google肯定還會(huì)有新內(nèi)容放出來，歡迎想?yún)⑴c貢獻(xiàn)的小伙伴加入，也歡迎在github上加star。

這篇文章是Google’s Engineering Practices documentation的第一章Code Review實(shí)踐指南

谷歌Code Review指南, 包含兩個(gè)子章節(jié)：

評審者指南

開發(fā)者指南

術(shù)語

部分文檔中會(huì)用到一些谷歌內(nèi)部的術(shù)語，特在此說明:

CL: “changelist"的縮寫，代表已經(jīng)進(jìn)入版本控制軟件或者正在進(jìn)行代碼評審的變更。

其他組織經(jīng)常稱為"change"或者"patch”。

LGTM: "Looks Good to Me."的縮寫，看起來不錯(cuò)，評審者批準(zhǔn)CL時(shí)會(huì)這么說。

評審者指南

Code Review標(biāo)準(zhǔn)

Code Review的主要目的是始終保證隨著時(shí)間的推移，谷歌代碼越來越健康，所有Code Review的工具和流程也是針對于此設(shè)計(jì)的。

為了完成這點(diǎn)，我們不得不權(quán)衡利弊。

首先，開發(fā)者應(yīng)當(dāng)在他們代碼中做一些 改進(jìn) ，如果你永遠(yuǎn)都不做出改進(jìn)，代碼庫整體質(zhì)量也不會(huì)提升。但是如果審查者為難所有變更，開發(fā)者未來也會(huì)失去改進(jìn)的動(dòng)力。

另一方面，保證代碼庫隨時(shí)間推移越來越健康是審查者的責(zé)任，而不是讓代碼庫質(zhì)量變得越來越差。這很棘手，因?yàn)榇a質(zhì)量一般都會(huì)隨著時(shí)間推移越來越差，尤其是在團(tuán)隊(duì)有明確時(shí)間限制、而且他們覺得不得不采取一些投機(jī)取巧的方式才能完成任務(wù)的情況下。

但是，代碼評審者得對他們Review的代碼負(fù)責(zé)，所以他們想始終確保代碼庫一致、可維護(hù)（其他指標(biāo)見"Code Review應(yīng)該關(guān)注什么？"）

依據(jù)這些，我們將以下準(zhǔn)則作為我們期望的Code Review標(biāo)準(zhǔn)：

通常而言，只要代碼對系統(tǒng)有明顯的提升且正常工作，即便不完美，評審者也應(yīng)該傾向于通過這次變更。

這是所有Code Review指南中的高級原則。

當(dāng)然這也有些局限。例如，如果變更里加入了有些評審者在系統(tǒng)里不想要的功能，即便代碼設(shè)計(jì)的很好，評審者也可以拒絕掉。

沒有完美無缺的代碼，只有越來越好的代碼。代碼評審者絕不應(yīng)該要求開發(fā)者打磨好CL中的每個(gè)細(xì)節(jié)才予以通過，相反，評審者應(yīng)該權(quán)衡項(xiàng)目進(jìn)度和他們給出建議的重要性，適當(dāng)放寬要求。評審者應(yīng)該追求 持續(xù)提高 ，而不是追求完美。那些可以提升整個(gè)系統(tǒng)可維護(hù)性、可讀性和可以理解性的變更不應(yīng)該因?yàn)榇a不夠完美而被推遲幾天甚至幾周。

評審者要 始終 不拘謹(jǐn)于在代碼評論里提示可以更好的想法。 但如果不是很重要信息，可以在評論前面加上標(biāo)識(shí)告訴他們可以忽略。

注意：這篇文檔中沒有任何地方辯解在變更中的檢查會(huì)讓整個(gè)系統(tǒng)代碼變得 更糟糕 。你唯一能做的在緊急狀況中說明。

指導(dǎo)性

Code Review有個(gè)重要的作用，那就是可以教會(huì)開發(fā)者關(guān)于語言、框架或者通用軟件設(shè)計(jì)原理。在Code Review中留下評論來幫助開發(fā)者學(xué)習(xí)新東西是很值得提倡的，畢竟共享知識(shí)也是長期提升系統(tǒng)代碼健康度的一部分。但請注意，如果你的評論純粹是教育性的，并且不是這篇文檔中提到的關(guān)鍵標(biāo)準(zhǔn)，請?jiān)谇懊婕由稀癗it:”標(biāo)識(shí)，或者明確指出不需要在這次變更中解決。

原則

技術(shù)和數(shù)據(jù)高于意見和個(gè)人偏好。

如果沒有其他的規(guī)則使用，只要保證不會(huì)影響系統(tǒng)的健康度，評審者可以要求開發(fā)者保持和現(xiàn)有的代碼庫一致。

解決代碼沖突

如果Code Review中有任何沖突，開發(fā)人員和評審人員都應(yīng)該首先根據(jù)開發(fā)者指南和評審者指南中其他文檔的內(nèi)容，嘗試達(dá)成一致意見。

當(dāng)很難達(dá)成一致時(shí)，開發(fā)者和評審者不應(yīng)該在Code Review評論里解決沖突，而是應(yīng)該召開面對面會(huì)議或者找個(gè)權(quán)威的人來協(xié)商。（如果你在評論里協(xié)商，確保在評論里記錄了討論結(jié)果，以便日后其他人翻閱。）

如果這樣都解決不了問題，那解決問題的方式就應(yīng)該升級了。通常的方式是拉著團(tuán)隊(duì)一起討論、讓團(tuán)隊(duì)主管來權(quán)衡、參考代碼維護(hù)者的意見，或者讓管理層來決定。不要因?yàn)殚_發(fā)者和評審者不能達(dá)成一致而把變更一直放在那里。

Code Review應(yīng)該關(guān)注什么？

注意:當(dāng)我們考慮以下點(diǎn)時(shí)，應(yīng)當(dāng)始終遵循Code Review標(biāo)準(zhǔn)。

設(shè)計(jì)

Code Review中最重要的一個(gè)點(diǎn)就是把握住變更中的整體設(shè)計(jì)。變更中各個(gè)部分的代碼交互是否正常？整個(gè)改動(dòng)是否屬于你負(fù)責(zé)的代碼庫？是否和你系統(tǒng)中其他部分交互正常？現(xiàn)在是否是添加整個(gè)功能的恰當(dāng)時(shí)間？

功能性

開發(fā)者在這個(gè)變更中想做什么？ 開發(fā)人員打算為該代碼的用戶帶來什么好處？（這里”用戶“是指受變更影響到的實(shí)際用戶，和將來會(huì)使用到這些代碼的開發(fā)者）

重要的是，我們希望開發(fā)者能充分測試代碼，以確保代碼在Code Review期間正常運(yùn)行。但無論如何，你作為審查者也要考慮一些特殊情況，像是有些并發(fā)問題。站在用戶的角度，確保你正在看的代碼沒有bug。

你可以驗(yàn)證變更，尤其是在有面向用戶的影響時(shí)，評審者應(yīng)該仔細(xì)檢查整個(gè)變更。有時(shí)候單純看代碼很難理解這個(gè)變更如何影響到用戶，這種情況下如果你不方便自己在CL中打補(bǔ)丁并親自嘗試，你可以讓開發(fā)者為你提供一個(gè)功能性的demo。

另一個(gè)在Code Review時(shí)需要特別關(guān)注的點(diǎn)是，CL中是否有 并發(fā)編程，并發(fā)理論上可能會(huì)導(dǎo)致死鎖或資源爭搶，這種問題在代碼運(yùn)行時(shí)很難被檢測出來，所以需要有人（開發(fā)者和評審者）仔細(xì)考慮整個(gè)邏輯，以確保不會(huì)引入線程安全的問題。（所以除了死鎖和資源爭搶之外，增加Code Review復(fù)雜度也可以成為拒絕使用多線程模型的一個(gè)理由。）

復(fù)雜性

變更是否比預(yù)期的更復(fù)雜？檢測變更的每個(gè)級別，是否個(gè)別行太復(fù)雜？功能是否太復(fù)雜？類是否太復(fù)雜？”復(fù)雜“意味著代碼閱讀者很難快速理解代碼，也可意味著開發(fā)者嘗試調(diào)用或修改此代碼時(shí)可能會(huì)引入bug。。

一個(gè)典型的復(fù)雜性問題就是 過度設(shè)計(jì)，當(dāng)開發(fā)者讓代碼變得更通用，或者增加了許多當(dāng)前不需要的功能時(shí)，評審者應(yīng)該額外注意是否過度設(shè)計(jì)。鼓勵(lì)開發(fā)者解決現(xiàn)在遇到的問題，而不是揣測未來可能遇到的問題。未來的問題應(yīng)當(dāng)在遇到時(shí)解決，到那個(gè)時(shí)候你才能看到問題本質(zhì)和實(shí)際需求。

測試

開發(fā)人員應(yīng)當(dāng)進(jìn)行單元測試、集成測試或端到端測試。一般來說，變更中應(yīng)該包含測試，除非這個(gè)變更只是為了處理緊急情況。

確保變更中的測試是正確、合理和有用的。因?yàn)闇y試本身無法測試自己，而且我們很少會(huì)為測試編寫測試，所以必須確保測試是有效的。

如果代碼出了問題，測試會(huì)失敗嗎？如果代碼發(fā)生改動(dòng)，它們會(huì)誤報(bào)嗎？每一個(gè)測試都有斷言嗎？是否按照不同的測試方法對測試進(jìn)行分類？

記住，不要以為測試不是二進(jìn)制中的一部分就不關(guān)注其復(fù)雜度。

命名

開發(fā)人員是否使用了良好的命名方式？好的命名要能夠充分表達(dá)一個(gè)項(xiàng)（變量、類名等）是什么或者用來做什么，但又不至于讓人難以閱讀。

注釋

開發(fā)者有沒有寫出清晰易懂的注釋？所有的注釋都是必要的嗎？ 通常注釋應(yīng)該解釋清楚為什么這么做，而不是做了什么。如果代碼不清晰，不能清楚地解釋自己，那么代碼可以寫的更簡單。當(dāng)然也有些例外（比如正則表達(dá)式和復(fù)雜的算法，如果能夠解釋清楚他們在做什么，肯定會(huì)讓閱讀代碼的人受益良多），但大多數(shù)注釋應(yīng)該指代碼中沒有包含的信息，比如代碼背后的決策。

變更中附帶的其他注釋也很重要，比如列出一個(gè)可以移除的待辦事項(xiàng)，或者一個(gè)不要做出代碼變更的建議，等等。

注意，注釋不同于類、模塊或函數(shù)文檔，文檔是為了說明代碼片段如何使用和使用時(shí)代碼的行為。

風(fēng)格

在谷歌內(nèi)部，主流語言甚至部分非主流語言都有編碼風(fēng)格指南 ，確保變更遵循了這些風(fēng)格規(guī)范。

如果你想對風(fēng)格指南中沒有提及的風(fēng)格做出改進(jìn)，可以在注釋前面加上“Nit:”，讓開發(fā)人員知道這是一個(gè)你認(rèn)為可以改進(jìn)的地方，但不是強(qiáng)制性的。但請不要只是基于個(gè)人偏好來阻止變更的提交。

開發(fā)人員不應(yīng)該將風(fēng)格變更與其他變更放在一起，這樣很難看出變更里有哪些變化，讓合并和回滾變得更加復(fù)雜，也會(huì)導(dǎo)致更多的其他問題。如果開發(fā)者想要重新格式化整個(gè)文件，讓他們將重新格式化后的文件作為單獨(dú)的變更，并將功能變更作為另一個(gè)變更。

文檔

如果變更改變了用戶構(gòu)建、測試、交互或者發(fā)布代碼相關(guān)的邏輯，檢測是否也更新了相關(guān)文檔，包括READMEs, g3doc頁面，以及任何相關(guān)文檔。如果開發(fā)者刪除或者棄用某些代碼，考慮是否也應(yīng)該刪除相關(guān)文檔。如果文檔有確實(shí)，讓開發(fā)者補(bǔ)充。

代碼細(xì)節(jié)

查看你整個(gè)Code Review中的每一行代碼，比如你可以掃到的數(shù)據(jù)文件，生成的代碼或大型數(shù)據(jù)結(jié)構(gòu)，但不要只掃一遍手寫的類，函數(shù)或者代碼塊，然后假設(shè)它們都能正常運(yùn)行。顯然，有些代碼需要仔細(xì)檢查，這完全取決于你，但你至少應(yīng)該理解所有的代碼都在做什么。

如果你很難看懂代碼，導(dǎo)致Code Review的速度慢了下來，你要讓開發(fā)者知道，并且在Review前澄清原因。在谷歌，我們有最優(yōu)秀的工程師，你也是其中之一。如果你都看不懂，很可能其他人也看不懂。所以要求開發(fā)者理清代碼邏輯也是在幫助未來的開發(fā)者。

如果你理解代碼，但又覺得沒有資格做代碼評審，確保有資格的變更評審人員在代碼評審時(shí)考慮到了安全性、并發(fā)性、可訪問性、國際化等問題。

上下文

看改動(dòng)上下文代碼對Code Review很有幫助，因?yàn)橥ǔode Review工具只會(huì)顯示改動(dòng)部分上下幾行代碼，但有時(shí)你必須查看整個(gè)文件確保這次變更可以正常運(yùn)行。例如，你可能看到加了4行代碼，但是看到整個(gè)文件時(shí)才會(huì)發(fā)現(xiàn)這加的4行代碼是在一個(gè)50多行的方法里，這個(gè)方法現(xiàn)在應(yīng)該被拆分為多個(gè)小的方法了。

Code Review時(shí)考慮到整個(gè)系統(tǒng)的上下文也很重要，這次改動(dòng)提升了系統(tǒng)健康度？或者增加了系統(tǒng)復(fù)雜性？少了測試用例？…… 不要通過哪些會(huì)損害系統(tǒng)健康的代碼。 很多系統(tǒng)變復(fù)雜都是因?yàn)橐稽c(diǎn)一點(diǎn)的小改動(dòng)日積月累造成的，所以千萬不要放進(jìn)來任何增加復(fù)雜性的改動(dòng)。

亮點(diǎn)

如果你看到變更中做的好的地方，告訴開發(fā)者他們做的很棒，尤其是當(dāng)他們用某種很精巧的方式解決你評論中提到的問題時(shí)更應(yīng)不吝贊美。 Code Review過多關(guān)注于錯(cuò)誤，但你也應(yīng)該為開發(fā)者展現(xiàn)出好的一面。在指導(dǎo)他人的時(shí)候，有時(shí)候告訴開發(fā)者正確的做法比告訴他們錯(cuò)誤的做法更有價(jià)值。

總結(jié)

在做Code Review時(shí)，你需要注意以下：

代碼設(shè)計(jì)良好。

代碼功能對用戶有用。

任何UI改動(dòng)應(yīng)當(dāng)是深思熟慮過而且看起來不錯(cuò)的。

保證線程安全。

代碼沒有增加不必要的復(fù)雜性。

開發(fā)者沒有寫有些將來需要但現(xiàn)在不知道是否需要的東西。

代碼有適當(dāng)?shù)膯卧獪y試。

測試邏輯設(shè)計(jì)良好。

開發(fā)者使用了清晰明了的命名。

注釋清晰明了實(shí)用，通常解釋清楚了為什么這么做，而不是做了啥。

代碼又相應(yīng)完善的文檔。

代碼風(fēng)格符合規(guī)范。

確保你review了要求你看的每一行代碼，確保你正在提升代碼質(zhì)量，并且為開發(fā)者做的提升。

Code Review步驟

概要

現(xiàn)在你知道了要從CL中得到什么，那能在眾多文件中完成評審的方法是什么？

這條改動(dòng)是否生效？這次變更是不是描述清晰?

首先閱讀CL最重要的一部分，整體上是否設(shè)計(jì)良好？

按照合適的順序閱讀剩下的變更。

第一步: 綜觀這個(gè)改動(dòng)

閱讀CL描述并且明白CL大體內(nèi)容。這些修改是否有意義？首先如果這個(gè)修改不應(yīng)該有，請立刻說明為什么這些修改不應(yīng)該有。當(dāng)你因?yàn)檫@個(gè)拒絕了這次改動(dòng)提交時(shí)，告訴開發(fā)人員該怎么去做是非常好的。

例如，您可能會(huì)說：“看起來您為此做了一些出色的工作，謝謝！但是，我們實(shí)際上正著手刪除FooWidget系統(tǒng)，您正在此處進(jìn)行修改，因此我們不想進(jìn)行任何新的修改現(xiàn)在。所以，您可以重構(gòu)我們的新BarWidget類嗎？"

請注意，審閱者不僅拒絕了當(dāng)前的CL并提供了替代建議，但他們禮貌地做到了。這種禮貌是重要，因?yàn)槲覀兿氡砻魑覀兩踔猎陂_發(fā)人員之間也相互尊重，尤其是當(dāng)我們意見不同時(shí)。

如果您得到的多個(gè)CL并且您不想進(jìn)行的更改，您應(yīng)該考慮重新設(shè)計(jì)團(tuán)隊(duì)的開發(fā)流程或發(fā)布的外部貢獻(xiàn)者的流程，以便在CL完成之前進(jìn)行更多的交流。最好在做大量工作之前告訴人們“不必做”，因?yàn)楝F(xiàn)在要將其丟棄或徹底重寫。

第二步: 檢查CL的主要部分

查找屬于此CL的“主要”部分的文件。通常來說一個(gè)CL最重要的部分是它邏輯修改數(shù)最多的那個(gè)文件。這樣有助于了解相關(guān)的CL，通常來說會(huì)加快code review。如果CL太大，您無法確定哪些部分是主要部分，請開發(fā)人員告訴你應(yīng)該首先看什么，或要求他們將CL拆分為多個(gè)CL。

如果您發(fā)現(xiàn)CL的這一部分存在一些主要的設(shè)計(jì)問題，則即使您現(xiàn)在沒有時(shí)間審查CL的其余部分，也應(yīng)立即寫下這些評注。 實(shí)際上，檢查其余的CL可能會(huì)浪費(fèi)時(shí)間，因?yàn)槿绻O(shè)計(jì)問題足夠嚴(yán)重，那么許多其他正在檢查的代碼將消失并且無論如何都不會(huì)發(fā)生。

立即寫下這些主要設(shè)計(jì)評注非常重要有兩個(gè)主要原因：

開發(fā)人員通常會(huì)發(fā)送給審核者一個(gè)CL，然后在等待審核時(shí)立即基于該CL進(jìn)行新工作。 如果您正在審查的CL中存在重大設(shè)計(jì)問題，那么他們也將不得不重新設(shè)計(jì)其以后的CL。你能在他們做太多無用功之前制止他們。

重要的設(shè)計(jì)變更比小的變更需要更長的時(shí)間。 開發(fā)人員幾乎都有截止日期；為了在截止日期之前完成工作， 并在代碼庫中保留高質(zhì)量的代碼，開發(fā)人員需要盡快開始CL的所有主要的重做。

第三步:依序閱讀CL的其余部分

確認(rèn)CL整體上沒有大的設(shè)計(jì)問題后，請嘗試找出邏輯順序來瀏覽文件，同時(shí)還要確保不要錯(cuò)過對任何文件的審查。通常，在瀏覽了主要文件之后，按照代碼審查工具向您展示它們的順序?yàn)g覽每個(gè)文件是最簡單的。有時(shí)在閱讀主要代碼之前先閱讀測試代碼也是有幫助的，因?yàn)檫@樣您就可以知道更改應(yīng)該做什么。

Code Review的速度

為什么Code Review應(yīng)該快？

在谷歌內(nèi)部，我們在持續(xù)優(yōu)化團(tuán)隊(duì)開發(fā)新產(chǎn)品的速度，而不是優(yōu)化單個(gè)開發(fā)人員編寫代碼的速度。 個(gè)人開發(fā)的速度固然重要，但它沒有團(tuán)隊(duì)整體的速度那么重要。

如果Code Review速度慢了，可能會(huì)發(fā)生以下這些事：

整個(gè)團(tuán)隊(duì)的速度會(huì)降低，是的，你不快速響應(yīng)別人的Code Review也可以完成其他的工作。但是，整個(gè)團(tuán)隊(duì)的新功能、bug修復(fù)可能會(huì)因?yàn)闆]有人做cr被延遲幾天、幾周甚至幾個(gè)月。

開發(fā)者應(yīng)維護(hù)Code Review的流程，如果審查者很少回復(fù)Code Review，但是每次都對CL提出大量的改動(dòng)，這可能會(huì)打擊到開發(fā)者。通常，開發(fā)者可能會(huì)抱怨審查者太嚴(yán)格。如果審查者能在開發(fā)者更新后快速響應(yīng)，并提出有實(shí)質(zhì)性提升的建議（能顯著提升代碼運(yùn)行狀況的CL），抱怨就會(huì)消失。 Code Review中絕大多數(shù)抱怨會(huì)隨著CR速度的提升而消失。

可能影響到代碼質(zhì)量。 如果CR慢了，可能會(huì)給開發(fā)者一種需要提交不太好代碼的壓力。CR慢了，也會(huì)影響到代碼清理、重構(gòu)、和現(xiàn)有CL的進(jìn)一步提升。

應(yīng)該以什么樣的速度做Code Review？

如果你不是在做需要高度專注的任務(wù)，Code Review應(yīng)該越快越好

應(yīng)當(dāng)在一個(gè)工作日之內(nèi)回應(yīng)Code Review請求。

遵循這些指導(dǎo)意味著典型的CL應(yīng)該在一天內(nèi)進(jìn)行多輪審查（如果需要）。

速度 vs 中斷

只有一種情況下，個(gè)人的速度要?jiǎng)龠^團(tuán)隊(duì)速度。如果您正在處理諸如編寫代碼之類的重要工作，請不要打斷自己的工作去做Code Review，研究人在專注中被打斷后需要很長的時(shí)間才能重新恢復(fù)到專注狀態(tài)。所以，為了不讓其他開發(fā)者等會(huì)而且中斷自己的編碼工作，明顯得不償失。

所以，建議你在工作斷點(diǎn)的時(shí)候回應(yīng)Code Review，比如寫完代碼、午飯后、會(huì)議結(jié)束后、從茶水間回來……

快速響應(yīng)

當(dāng)我們談?wù)揅ode Review的速度時(shí)，一方面是指響應(yīng)時(shí)間，另一方面是指整個(gè)Review從提交到通過的時(shí)間。整個(gè)過程應(yīng)該足夠快，但是對于每個(gè)人來說，迅速做出反應(yīng)比迅速完成整個(gè)過程更為重要。

即使有時(shí)需要很長時(shí)間才能完成整個(gè)Code Review流程，評審者在整個(gè)過程中的快速響應(yīng)也可以大大緩解開發(fā)人員因?yàn)镃ode Review“慢”而產(chǎn)生的挫敗感。

如果你太忙不能全身心投入到Code Review中，你也應(yīng)該讓開發(fā)者知道你什么時(shí)候會(huì)去Review，或者建議其他評審者快速響應(yīng)，再或者提供一些初步的建議。（注意：這不是建議你中斷自己的工作，而是在工作間隙合理響應(yīng)）

評審者有必要花時(shí)間去做Code Review來保證代碼符合標(biāo)準(zhǔn)，不管怎么樣，每個(gè)回應(yīng)應(yīng)當(dāng)保證足夠快速。

跨時(shí)區(qū)Code Review

LGTM和注釋

為了讓CR更快，有些情況下也應(yīng)該讓CR提前通過，即便有些評論沒有被解決，比如：

審查者信任開發(fā)者能適當(dāng)解決所有評審者的建議。

其余的改動(dòng)很小，開發(fā)者不必做。

除非另有明確說明，評審者應(yīng)指明他們打算使用這些選項(xiàng)中的哪一個(gè)。

當(dāng)開發(fā)者和評審者在不同時(shí)區(qū)時(shí)，應(yīng)當(dāng)著重考慮下通過CR，否則開發(fā)者還得等一天。

大的CL

如果有人給你提交了一個(gè)非常大的Code Review，你也不確定你有時(shí)間看，你最好建議開發(fā)者把CL拆分成幾個(gè)小的部分，分多次Code Review，而不是一次性全部提交上來。這即便開發(fā)者多做一些額外的工作，也是可以把它拆分開的，而且拆分也有利于評審者。

如果CL不能拆分成多個(gè)小CL，你也沒有時(shí)間快速完整的Review整個(gè)代碼，只是對整體設(shè)計(jì)提一些建議，然后發(fā)回給開發(fā)者改進(jìn)。作為評審者，你的目標(biāo)之一是在不犧牲代碼質(zhì)量的前提下，不阻礙開發(fā)者的進(jìn)程或者盡可能讓他們向前推進(jìn)。

持續(xù)提升Code Review

如果你遵從這些建議并在Code Review中嚴(yán)格執(zhí)行這些準(zhǔn)則，你就會(huì)發(fā)現(xiàn)整個(gè)Code Review的流程會(huì)越來越快。 開發(fā)者將了解健康代碼的要求，并從一開始就交出完美的代碼，然后Code Review的時(shí)間會(huì)越來越少。評審者將學(xué)會(huì)如何快速做出響應(yīng)，并且不是在這個(gè)Code Review過程中增加不必要的延遲。

但是，不要為了提升速度犧牲Code Review的標(biāo)準(zhǔn)和代碼質(zhì)量。 從長遠(yuǎn)來看，這并不會(huì)提升速度。

緊急情況

當(dāng)然也有一些緊急的CL需要快速走完這個(gè)Code Review流程，這時(shí)候在質(zhì)量上的把控可以稍微放松一些。可以參考緊急事件一文來了解哪些是緊急事件哪些不是。

怎樣寫代碼評論

概要

禮貌

解釋你的觀點(diǎn).

明確指出方向和問題，幫助開發(fā)人員去權(quán)衡作出決定.

鼓勵(lì)開發(fā)人員通過注釋和精簡代碼來解決你的困惑而不是通過解釋

禮貌

通常來說當(dāng)你Code Review代碼時(shí)保持禮貌和尊重能使開發(fā)人員更加清晰，得到更多幫助。這樣是為了保證你的代碼評論僅僅針對的是code而不是針對開發(fā)人員。你不必一直這么去做，但是當(dāng)你的評論會(huì)讓開發(fā)人員生氣或者產(chǎn)生爭執(zhí)時(shí)有必要這么去做。比如：

不好的例子: “你為什么會(huì)在這里使用線程，這樣做難道會(huì)有任何好處？”

好的例子: "我并沒有發(fā)現(xiàn)這個(gè)并發(fā)模塊給程序帶來了多少幫助，并且還增加了程序的復(fù)雜性，因此我認(rèn)為這段代碼最好是用單線程而不是多線程。

解釋清楚原因

從上面“好”的例子當(dāng)中你能發(fā)現(xiàn)，這樣有助于開發(fā)人員理解為什么你寫了這些評注。你不一定非得包含這些信息在你的評注里面，但是適當(dāng)?shù)亩嘟忉屇愕囊鈭D或者多給出一些提升代碼質(zhì)量的建議都是非常好的實(shí)踐。

給予指導(dǎo)

通常來說修復(fù)CL是開發(fā)人員的職責(zé)而不是評審人員的。你不需要向開發(fā)人員提供詳細(xì)的解決方案或者代碼。

但是這并不意味著評審員就不應(yīng)該提供幫助。你需要在指出問題和提供直接指導(dǎo)之間找到平衡。指出問題并且?guī)椭_發(fā)人員決策能夠幫助開發(fā)人員學(xué)同事讓code review變得更加簡單。這樣也能產(chǎn)生更好的方案，因?yàn)殚_發(fā)人員比評審者更加了解代碼。

盡管這樣，有時(shí)候直接給出指導(dǎo)，建議甚至是代碼更有幫助。code review的主要目的是盡可能得到最好的CL。其次是提高開發(fā)人員的技能這樣就能減少以后評審的次數(shù)。

接受解釋

與其要求讓開發(fā)人員解釋一段你看不懂的代碼，其實(shí)更應(yīng)該做的是讓他們重寫代碼，讓代碼更清晰。當(dāng)一段代碼不是太過于復(fù)雜的時(shí)候通過加一些注釋偶爾也是一種不錯(cuò)的做法。

**解釋僅僅是寫在Code Review工具里的，不利于將來的代碼閱讀者。**只有極少數(shù)情況是可行的，比如你對你評審的需求不太熟悉，但是開發(fā)人員解釋的是大多數(shù)人都知道的。

處理Code Review中的反駁

有時(shí)開發(fā)者會(huì)在Code Review中反駁你，他們可能不同意你的意見，或者抱怨你太嚴(yán)格了。

誰是誰非？

當(dāng)開發(fā)者不同意你的建議時(shí)，首先花點(diǎn)思考下他們是否是對的，但通常而言你比他們更熟悉代碼，所以可能在某個(gè)方面理解更深。他們的爭論有意義嗎？從代碼健康的角度來看他們的反駁有意義嗎？如果是，讓他們知道他們是對的，然后這個(gè)問題就解決了。

然而，開發(fā)者不總是對的，這種情況下評審者應(yīng)當(dāng)進(jìn)一步介紹為什么他們的建議是對的。好的解釋不僅得體現(xiàn)出對開發(fā)人員回復(fù)的理解，而且還要說明為什么要這么改。

如果評審者認(rèn)為他們的建議可以改善代碼質(zhì)量，并且他們認(rèn)為帶來的代碼質(zhì)量改進(jìn)值得開發(fā)者做這些額外的工作，評審者就應(yīng)該堅(jiān)持自己的立場。

不積跬步無以至千里，不積小流無以成江河

有時(shí)候要讓開發(fā)者接受，你得花很多時(shí)間反復(fù)解釋，但始終確保該有的禮貌，并讓開發(fā)者知道在知道他們了什么，即便是你不同意。

惹惱開發(fā)者

有時(shí)評審者會(huì)認(rèn)為堅(jiān)持讓開發(fā)者做出改動(dòng)，可能會(huì)惹惱開發(fā)者。有時(shí)開發(fā)者確實(shí)會(huì)惱怒，但這種情況通常都很短暫，而且之后他們會(huì)感激你幫助他們提高了代碼質(zhì)量。 如果你在Code Review中很有禮貌，開發(fā)者根本不會(huì)被惹惱，這種擔(dān)心是多余的。通常，令惹惱開發(fā)者的是你寫注解的方式，而不是你對代碼質(zhì)量的堅(jiān)持。

稍后解決

一種常見的反駁原因是開發(fā)者希望能盡快完成任務(wù)。他們不想一輪又一輪地做Code Review，然后就會(huì)說他們會(huì)在后續(xù)CL中處理這些問題，你只需要通過就行。有些開發(fā)者做的很好，他們會(huì)立馬提交后續(xù)CL處理這些問題。然而，經(jīng)驗(yàn)告訴我們原始CL通過之后拖的時(shí)間越久，就越不可能修復(fù)。除非開發(fā)者在當(dāng)前CL通過后立馬修復(fù)，否則他們就不可能修復(fù)。這并不是開發(fā)者不負(fù)責(zé)任，而是因?yàn)樗麄冇泻枚喙ぷ饕觯迯?fù)工作也會(huì)因?yàn)楣ぷ鲏毫Χ贿z忘。所以最好堅(jiān)持讓開發(fā)者現(xiàn)在就在CL中處理掉這些問題，“留著以后清理”是一種不可取的方式。

如果CL中引入了新的復(fù)雜性，提交之前必須清理掉，除非是緊急情況。 如果CL中暴露出一些目前還無法定位的問題，開發(fā)者應(yīng)該記錄下這些bug，并將其分配給他們自己，確保這些問題不會(huì)被遺忘。他們還可以在代碼中加入 TODO 注釋，指向已經(jīng)記錄好的 bug。

抱怨太嚴(yán)格

如果你之前Code Review很寬容，然后突然變得嚴(yán)格起來，可能會(huì)引起一些開發(fā)者的抱怨。不過沒關(guān)系，加快Code Review的速度通常會(huì)讓這些抱怨消失。

有時(shí)，這些抱怨可能需要幾個(gè)月的時(shí)間才能消除，但最終開發(fā)者在看到產(chǎn)出的優(yōu)質(zhì)代碼時(shí)會(huì)理解嚴(yán)格Code Review帶來的價(jià)值。有時(shí)候，一旦發(fā)生某些事讓他們真正看到嚴(yán)格Code Review的價(jià)值，抗議最大聲的人甚至?xí)蔀槟阕顖?jiān)定的支持者。

解決沖突

如果你遵循了上述方法，但仍然會(huì)在Code Review中遇到無法解決的沖突，請?jiān)俅螀㈤咰ode Review標(biāo)準(zhǔn)，了解那些有助于解決沖突的指導(dǎo)和原則。

開發(fā)者指南

寫一個(gè)好的CL描述

一個(gè)CL描述是做了什么更改以及為什么的公開記錄。

它將成為我們版本控制歷史的永久組成部分，多年來除你的評審者以外，還可能有數(shù)百人會(huì)閱讀它。

將來的開發(fā)者將會(huì)基于它的描述來搜索你的CL。

將來可能會(huì)有人由于沒有現(xiàn)成的細(xì)節(jié)，而根據(jù)他模糊的記憶來尋找你的改變。

如果所有重要的細(xì)節(jié)都在代碼中而不是描述中，那么他們將很難定位你的CL。

第一行

言簡意賅

語義完整

空行隔開

CL描述的第一行應(yīng)該是對CL正在做的具體工作的簡短總結(jié)，緊跟一個(gè)空行。

在未來，這是大多數(shù)的代碼搜索者在瀏覽一段代碼的版本控制歷史時(shí)會(huì)看到的，因此第一行應(yīng)該足夠有信息量，他們不必閱讀你的CL或它的整個(gè)描述就可以大致了解你的CL實(shí)際做了什么。

按照傳統(tǒng)，CL描述的第一行是一個(gè)完整的句子，就像它是一個(gè)命令（一個(gè)祈使句）。

例如，說"Delete the FizzBuzz RPC and replace it with the new system."，而不是"Deleting the FizzBuzz RPC and replacing it with the new system."。

不過，你不必把其余的描述寫成祈使句。

正文內(nèi)容豐富

其余描述應(yīng)該是具有豐富的內(nèi)容。它可能包括對正在解決的問題的簡要描述，以及為什么這是最好的方法。 如果這種方法有任何缺點(diǎn)，就應(yīng)該提到。將相關(guān)的背景信息（例如bug數(shù)目，基準(zhǔn)測試結(jié)果），以及設(shè)計(jì)文檔的鏈接。即使是很小的CL也值得關(guān)注細(xì)節(jié)。請將來龍去脈放入CL中。

反例

"Fix bug"是一個(gè)不充分的CL描述。是什么bug？你是怎么修復(fù)它的？

其他一些類似的不好的CL描述:

“Fix build.”

“Add patch.”

“Moving code from A to B.”

“Phase 1.”

“Add convenience functions.”

“kill weird URLs.”

好的CL描述

這里有一些好的CL描述的例子.

rpc: remove size limit on RPC server message freelist.

Servers like FizzBuzz have very large messages and would benefit from reuse.

Make the freelist larger, and add a goroutine that frees the freelist entries

slowly over time, so that idle servers eventually release all freelist

entries.

前幾個(gè)詞描述了CL描述實(shí)際做了什么。其余描述在說明解決的問題，為什么這是一個(gè)好的方案，以及具體實(shí)現(xiàn)的細(xì)節(jié)。

Construct a Task with a TimeKeeper to use its TimeStr and Now methods.

Add a Now method to Task, so the borglet() getter method can be removed (which

was only used by OOMCandidate to call borglet’s Now method). This replaces the

methods on Borglet that delegate to a TimeKeeper.

Allowing Tasks to supply Now is a step toward eliminating the dependency on

Borglet. Eventually, collaborators that depend on getting Now from the Task

should be changed to use a TimeKeeper directly, but this has been an

accommodation to refactoring in small steps.

Continuing the long-range goal of refactoring the Borglet Hierarchy.

第一行描述了CL做了什么，以及它是如何與過去發(fā)生變化的。

其余的描述將討論具體的實(shí)現(xiàn)、CL的來龍去脈、解決方案的不理想以及未來可能的方向。

它同樣是解釋為什么要進(jìn)行這個(gè)修改。

Create a Python3 build rule for status.py.

This allows consumers who are already using this as in Python3 to depend on a

rule that is next to the original status build rule instead of somewhere in

their own tree. It encourages new consumers to use Python3 if they can,

instead of Python2, and significantly simplifies some automated build file

refactoring tools being worked on currently.

第一句描述了實(shí)際做了什么。其余的描述解釋了為什么要進(jìn)行更改，并給了reviewer很多背景。

在提交CL之前，請檢查描述

在Review期間CL可能會(huì)經(jīng)歷重大改變。在提交CL之前，有必要review CL描述，以確保描述仍然反映CL所做的事情。

簡短化的CL

為什么要寫成一系列簡短的CL?

簡短的CL有這些好處:

Code Review更快 與比起花30分鐘審查一個(gè)大型的CL相比，對審查者來說花5分鐘審查一系列小的CL更加容易.

減少導(dǎo)致bug的可能性。 由于您所做的更改較少，因此您和您的審閱者更容易有效地推斷出CL的影響，并查看是否導(dǎo)致bug。

減少不必要的工作 當(dāng)你寫了一個(gè)巨大的CL，然后審查者覺得你總體方向錯(cuò)了，這會(huì)導(dǎo)致你浪費(fèi)大量的工作。

更方便合并代碼 因?yàn)榇笮偷腃L會(huì)導(dǎo)致大量的沖突，因此合并大型的CL會(huì)浪費(fèi)很多時(shí)間，并且這將會(huì)是你經(jīng)常做的工作。

有助于你作出更好的設(shè)計(jì) 優(yōu)雅的設(shè)計(jì)并且完善一個(gè)小的改動(dòng)比大的改動(dòng)更加容易

降低審查者的難度 提審部分改動(dòng)，不會(huì)影響你繼續(xù)編碼。

回滾更容易 大型CL很有可能會(huì)在初始CL提交和回滾CL之間更新修改文件，從而使回滾變得復(fù)雜（中型CL也可能也需要回滾可能也會(huì)這樣）。

注意！ 審查者可以因?yàn)槟愕母膭?dòng)過于巨大直接拒絕掉你通常，他們會(huì)感謝您的貢獻(xiàn)，但要求您以某種方式使它成為一系列較小的更改。不管是你把這些改動(dòng)拆分成小的改動(dòng)，還是和審查者爭論讓他接受都會(huì)耗費(fèi)你大量的時(shí)間。但是編寫小型改動(dòng)就不會(huì)有這樣的問題。

怎么樣算簡短?

通常，CL的正確大小是一個(gè)獨(dú)立的更改。 這意味著：

CL所做的最小更改僅解決了一件事情。 通常，這只是功能的一部分，而不是一次完整的功能。 通常，寧可編寫過小的CL也不要寫太大的CL。你可以和你的審查者商量找到合適的尺度。

審閱者需要了解的有關(guān)CL的所有內(nèi)容（將來的開發(fā)除外）都在CL，CL的描述，現(xiàn)有代碼庫或他們已經(jīng)查看過的CL中。

檢入CL后，系統(tǒng)將繼續(xù)對其用戶和開發(fā)人員正常運(yùn)行。

CL不夠小的話會(huì)導(dǎo)致其難以理解。如果你新增加了api，那么在同一個(gè)CL應(yīng)該包括這個(gè)api用到的方法，以便審查者更好的理解。也能防止檢入沒有用的api。

多大算大，沒有一個(gè)明確的要求。 對于CL而言，100行通常是一個(gè)合理的大小，而1000行通常太大，但這取決于您的審閱者的判斷。 更改分布的文件數(shù)量也會(huì)影響其“大小”。 可以在一個(gè)文件中進(jìn)行200行的更改，但是將其分布在50個(gè)文件中通常會(huì)太大。

請記住，盡管從您開始編寫代碼起就一直與您緊密聯(lián)系，但審閱者通常沒有上下文。 對您來說，看起來像可接受大小的CL可能會(huì)讓您的審閱者不知所措。毫無疑問，盡可能把CL些小是正確的。審查者很少抱怨CL太小

什么時(shí)候大型的CL也是可以的?

在某些情況下，較大的更改沒有那么糟糕:

通常，您可以將整個(gè)文件的刪除視為更改的一行，因?yàn)樗粫?huì)花費(fèi)很長時(shí)間來審閱審閱者。

有時(shí)，您完全信任的自動(dòng)重構(gòu)工具已經(jīng)生成了一個(gè)較大的CL，而審閱者的工作只是檢查健全性，然后指出他們認(rèn)為需要修改的地方。 這些CL可能更大，盡管會(huì)有一些風(fēng)險(xiǎn)（例如合并和測試）仍然適用。

拆分CL的另一種方法是通過將文件分組，如果這些文件將是獨(dú)立的更改，可以分配各不同的審閱者。

比如: 你提交一個(gè)修改的CL，創(chuàng)建一個(gè)修改的緩沖區(qū)，另一個(gè)CL的代碼修改也可以提交到這里面。你必須按照順序提交CL，但是審閱者可以同時(shí)進(jìn)行審閱. 如果這么做，你需要盡可能告訴所有審閱者另一個(gè)CL的信息，以便他們能得到上線文信息。

另一個(gè)示例：您發(fā)送一個(gè)CL進(jìn)行代碼更改，而另一個(gè)CL則發(fā)送使用該代碼的配置或?qū)嶒?yàn)； 如果需要，這也更容易回滾，因?yàn)橛袝r(shí)將配置/實(shí)驗(yàn)文件推送到生產(chǎn)環(huán)境中比更改代碼更快。

把代碼重構(gòu)分離出來

通常重構(gòu)最好不要和功能修改或者bug fix一起提CL。比如移動(dòng)或者重命名一個(gè)Class，最好和這個(gè)Class的bug fix分開提CL。這樣對于審閱者來說，理解每個(gè)CL單獨(dú)引入的更改要容易得多。

不過一些小的代碼清理工作比如變量重命名可以包含在功能修改或者bug fix的CL種。 這取決于開發(fā)者和審閱者的判斷，當(dāng)然如果巨大的重構(gòu)包含在同一個(gè)CL中會(huì)大大增加審閱的難度。

將相關(guān)的測試代碼保存在同一CL中

避免將測試代碼拆分為單獨(dú)的CL。 驗(yàn)證代碼修改的測試應(yīng)該進(jìn)入相同的CL，即使這樣做會(huì)增加代碼行數(shù)。

但是根據(jù)重構(gòu)準(zhǔn)則，獨(dú)立的測試修改可以單獨(dú)寫入CL。比如

使用新測試驗(yàn)證先前存在的提交代碼。

重構(gòu)測試代碼（例如，引入輔助函數(shù)）。

引入更大的測試框架代碼（例如集成測試）。

不要破壞結(jié)構(gòu)

如果您有多個(gè)相互依賴的CL，則需要找到一種方法來確保在提交每個(gè)CL之后整個(gè)系統(tǒng)都能正常工作。 否則，可能破壞代碼結(jié)構(gòu)導(dǎo)致后面的開發(fā)者浪費(fèi)大量的時(shí)間等你的提交（如果這些CL提交出了問題，則更長的時(shí)間）。

小到不能再小

在寫大型CL之前，思考下是不是能夠?qū)⒅貥?gòu)分離出來，這是一個(gè)更加清晰的思路。多和你的隊(duì)友交流學(xué)習(xí)下他們對縮小CL的實(shí)踐和方法。

如果所有這些選項(xiàng)都失敗了（這種情況很少見），那么請事先獲得您的審閱者的同意，告訴他們一個(gè)巨大的CL將要來臨。 在這種情況下，審查過程會(huì)耗費(fèi)極其長的實(shí)踐，這樣請花費(fèi)更多的時(shí)間去寫測試代碼，避免引入bug。

如何處理評審者的評論

當(dāng)你提交了一個(gè)變更做Code Review時(shí)，很可能你都會(huì)收到評審者在變更中的評論。這里有一些處理這些評論的建議。

不要摻雜個(gè)人情感

Code Review的目標(biāo)是維護(hù)代碼庫和產(chǎn)品的質(zhì)量。如果評審者批評了你的代碼，可以理解為他們在幫你、幫整個(gè)代碼庫、甚至是幫整個(gè)公司，而不是攻擊你或者是質(zhì)疑你的能力。

有時(shí)候評審者會(huì)情緒低落，然后在評論中說出一些令人沮喪的話，雖然評審者這樣做不對，但作為開發(fā)者你應(yīng)當(dāng)有心理準(zhǔn)備。問問你自己，“評審者試圖與我交流的建設(shè)性意見是什么？” 然后照他們說的那些去做。

永遠(yuǎn)不要回應(yīng)充滿怒氣的評論，Code Review工具中違反職業(yè)禮儀的情況永遠(yuǎn)存在。如果你真的忍無可忍，建議先離開電腦也會(huì)兒，或者干一些其他的事，等心情平復(fù)下來再回復(fù)。

通常，如果評審者沒有以禮貌的方式提供反饋，請親自向他們解釋。如果你無法親自或者通過視頻和他們聯(lián)系，就向他們發(fā)一份私人郵件。 友好地告訴他們你不喜歡的事情以及你希望他們做些什么。 如果他們也以非建設(shè)性的方式對此私密討論做出回應(yīng)，或者沒有起到預(yù)期的作用，請酌情上報(bào)給您的經(jīng)理。如果他們已經(jīng)以不禮貌的方式回應(yīng)，或者沒有取得預(yù)期的效果，視情況匯報(bào)給你的經(jīng)理。

修復(fù)代碼

如果評審者說他們理解不了你代碼中的某些內(nèi)容，你首先應(yīng)該把代碼寫清晰。如果讓代碼更清晰，添加注釋來解釋清楚代碼的邏輯。 如果評論似乎毫無意義，那么您的答復(fù)應(yīng)該只是代碼查看工具中的解釋。

如果評審者無法理解你的某部分代碼，那邊可能未來的閱讀者也可能理解不了。在Code Review工具中回應(yīng)幫不了未來的讀者，但是代碼中的注釋可以。

自我反思

寫一個(gè)變更會(huì)花費(fèi)你很大的精力，提價(jià)Code Review時(shí)會(huì)感覺如釋負(fù)重，而且自己也相當(dāng)確定所有工作已經(jīng)做完了。所以當(dāng)評審者提出改進(jìn)建議時(shí)，你很容易認(rèn)為那些都是錯(cuò)的，或者認(rèn)為是評審者給你不必要的阻撓，再或者覺得評審者應(yīng)該讓你提價(jià)變更。 無論如何，不管你怎么想，花點(diǎn)時(shí)間回想下評審者給你的反饋有助于提升公司的代碼質(zhì)量。你始終問下自己“如果評審者是對的呢？”

如果你回答不了評審者的問題，那可能說明評審者的評論不夠清楚。

如果你認(rèn)真考慮過后依舊認(rèn)為你是對的，放心大膽地解釋清楚為什么你的方法對公司更有利。通常，評審者只是提供建議，并且希望你能思考出更好的方法。也許你已經(jīng)知道一些評審者不知道的關(guān)于用戶、代碼庫、或者變更，把這些都寫下來，給評審者更多的上下文信息，通常你都可以根據(jù)某些事實(shí)和評審者達(dá)成某些共識(shí)。

解決沖突

解決沖突的第一步，和你的評審者達(dá)成共識(shí)，如果無法達(dá)成共識(shí)，參閱Code Review的標(biāo)準(zhǔn)獲取更多內(nèi)容。

開發(fā)者

版權(quán)聲明：本文內(nèi)容由網(wǎng)絡(luò)用戶投稿，版權(quán)歸原作者所有，本站不擁有其著作權(quán)，亦不承擔(dān)相應(yīng)法律責(zé)任。如果您發(fā)現(xiàn)本站中有涉嫌抄襲或描述失實(shí)的內(nèi)容，請聯(lián)系我們jiasou666@gmail.com 處理，核實(shí)后本網(wǎng)站將在24小時(shí)內(nèi)刪除侵權(quán)內(nèi)容。

版權(quán)聲明：本文內(nèi)容由網(wǎng)絡(luò)用戶投稿，版權(quán)歸原作者所有，本站不擁有其著作權(quán)，亦不承擔(dān)相應(yīng)法律責(zé)任。如果您發(fā)現(xiàn)本站中有涉嫌抄襲或描述失實(shí)的內(nèi)容，請聯(lián)系我們jiasou666@gmail.com 處理，核實(shí)后本網(wǎng)站將在24小時(shí)內(nèi)刪除侵權(quán)內(nèi)容。