作者 | 帥昕 xindoo
責(zé)編 | 屠敏
出品 | CSDN 博客
我和幾個小伙伴一起翻譯了google前一段時間放出來的Google’s Engineering Practices documentation（https://github.com/google/eng-practices），翻譯后的GitHub倉庫：https://github.com/xindoo/eng-practices-cn，歡迎加star。目前只是翻譯完了，因為譯者水平有限，還需要審校。另外后續(xù)Google肯定還會有新內(nèi)容放出來，歡迎想?yún)⑴c貢獻(xiàn)的小伙伴加入，也歡迎在GitHub上加star。
這篇文章是Google’s Engineering Practices documentation的第一章Code Review實踐指南：https://xindoo.github.io/eng-practices-cn/review/。
谷歌Code Review指南, 包含兩個子章節(jié)：
-
評審者指南：https://xindoo.github.io/eng-practices-cn/review/reviewer/
-
開發(fā)者指南：https://xindoo.github.io/eng-practices-cn/review/developer/
1.術(shù)語
部分文檔中會用到一些谷歌內(nèi)部的術(shù)語，特在此說明:
-
CL: “changelist"的縮寫，代表已經(jīng)進(jìn)入版本控制軟件或者正在進(jìn)行代碼評審的變更。其他組織經(jīng)常稱為"change"或者"patch”。
-
LGTM: "Looks Good to Me."的縮寫，看起來不錯，評審者批準(zhǔn)CL時會這么說。
2.評審者指南
Code Review標(biāo)準(zhǔn)
Code Review的主要目的是始終保證隨著時間的推移，谷歌代碼越來越健康，所有Code Review的工具和流程也是針對于此設(shè)計的。
為了完成這點，我們不得不權(quán)衡利弊。
首先，開發(fā)者應(yīng)當(dāng)在他們代碼中做一些 改進(jìn) ，如果你永遠(yuǎn)都不做出改進(jìn)，代碼庫整體質(zhì)量也不會提升。但是如果審查者為難所有變更，開發(fā)者未來也會失去改進(jìn)的動力。
另一方面，保證代碼庫隨時間推移越來越健康是審查者的責(zé)任，而不是讓代碼庫質(zhì)量變得越來越差。這很棘手，因為代碼質(zhì)量一般都會隨著時間推移越來越差，尤其是在團(tuán)隊有明確時間限制、而且他們覺得不得不采取一些投機(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è)計的很好，評審者也可以拒絕掉。
關(guān)鍵點是沒有任何完美的代碼，只有更好的代碼。代碼評審者絕對不應(yīng)該要求開發(fā)者在批準(zhǔn)前對變更中的每一個小塊進(jìn)行打磨，相反，評審者應(yīng)該權(quán)衡向前推進(jìn)和他們采納他們建議二者的重要性。評審者應(yīng)該追求 持續(xù)提高 ，而不是追求完美。那些可以提升整個系統(tǒng)可維護(hù)性、可讀性和可以理解性的變更不應(yīng)該因為代碼不夠完美而被推遲幾天甚至幾周。
評審者要 始終 不拘謹(jǐn)于在代碼評論里提示可以更好的想法。但如果不是很重要信息，可以在評論前面加上標(biāo)識告訴他們可以忽略。
注意：這篇文檔中沒有任何地方辯解在變更中的檢查會讓整個系統(tǒng)代碼變得 更糟糕 。你唯一能做的在緊急狀況中說明。
指導(dǎo)性
Code Review有個重要的作用，那就是可以教會開發(fā)者關(guān)于語言、框架或者通用軟件設(shè)計原理。在Code Review中留下評論來幫助開發(fā)者學(xué)習(xí)新東西是很值得提倡的，畢竟共享知識也是長期提升系統(tǒng)代碼健康度的一部分。但請注意，如果你的評論純粹是教育性的，并且不是這篇文檔中提到的關(guān)鍵標(biāo)準(zhǔn)，請在前面加上“Nit:”標(biāo)識，或者明確指出不需要在這次變更中解決。
原則
-
技術(shù)和數(shù)據(jù)高于意見和個人偏好。
-
關(guān)于風(fēng)格問題, 風(fēng)格指南是絕對的權(quán)威。任何不在樣式指南中指出的樣式(比如空格等)都是個人偏好的問題。風(fēng)格應(yīng)該與現(xiàn)有的一致。如果沒有以前的風(fēng)格，就按作者的風(fēng)格來。
-
軟件設(shè)計從來不是純粹的代碼風(fēng)格或是個人偏好問題，它們是基于一些應(yīng)當(dāng)被權(quán)衡的規(guī)則而不僅僅是個人傾向。有時候也會有多種有效的選項，如果開發(fā)者能證明(通過數(shù)據(jù)或者原理)這些方法都同樣有效，那么評審者應(yīng)該接受作者的偏好，否則應(yīng)該遵從軟件設(shè)計標(biāo)準(zhǔn)。
-
如果沒有其他的規(guī)則使用，只要保證不會影響系統(tǒng)的健康度，評審者可以要求開發(fā)者保持和現(xiàn)有的代碼庫一致。
解決代碼沖突
如果Code Review中有任何沖突，開發(fā)人員和評審人員都應(yīng)該首先根據(jù)開發(fā)者指南和評審者指南中其他文檔的內(nèi)容，嘗試達(dá)成一致意見。
當(dāng)很難達(dá)成一致時，開發(fā)者和評審者不應(yīng)該在Code Review評論里解決沖突，而是應(yīng)該召開面對面會議或者找個權(quán)威的人來協(xié)商。（如果你在評論里協(xié)商，確保在評論里記錄了討論結(jié)果，以便日后其他人翻閱。）
如果這樣都解決不了問題，那解決問題的方式就應(yīng)該升級了。通常的方式是拉著團(tuán)隊一起討論、讓團(tuán)隊主管來權(quán)衡、參考代碼維護(hù)者的意見，或者讓管理層來決定。不要因為開發(fā)者和評審者不能達(dá)成一致而把變更一直放在那里。
Code Review應(yīng)該關(guān)注什么？
注意:當(dāng)我們考慮以下點時，應(yīng)當(dāng)始終遵循Code Review標(biāo)準(zhǔn)。
設(shè)計
Code Review中最重要的一個點就是把握住變更中的整體設(shè)計。變更中各個部分的代碼交互是否正常？整個改動是否屬于你負(fù)責(zé)的代碼庫？是否和你系統(tǒng)中其他部分交互正常？現(xiàn)在是否是添加整個功能的恰當(dāng)時間？
功能性
開發(fā)者在這個變更中想做什么？開發(fā)人員打算為該代碼的用戶帶來什么好處？（這里”用戶“是指受變更影響到的實際用戶，和將來會使用到這些代碼的開發(fā)者）
重要的是，我們希望開發(fā)者能充分測試代碼，以確保代碼在Code Review期間正常運行。但無論如何，你作為審查者也要考慮一些特殊情況，像是有些并發(fā)問題。站在用戶的角度，確保你正在看的代碼沒有bug。
你可以驗證變更，尤其是在有面向用戶的影響時，評審者應(yīng)該仔細(xì)檢查整個變更。有時候單純看代碼很難理解這個變更如何影響到用戶，這種情況下如果你不方便自己在CL中打補(bǔ)丁并親自嘗試，你可以讓開發(fā)者為你提供一個功能性的demo。
另一個在Code Review時需要特別關(guān)注的點是，CL中是否有 并發(fā)編程，并發(fā)理論上可能會導(dǎo)致死鎖或資源爭搶，這種問題在代碼運行時很難被檢測出來，所以需要有人（開發(fā)者和評審者）仔細(xì)考慮整個邏輯，以確保不會引入線程安全的問題。（所以除了死鎖和資源爭搶之外，增加Code Review復(fù)雜度也可以成為拒絕使用多線程模型的一個理由。）
復(fù)雜性
變更是否比預(yù)期的更復(fù)雜？檢測變更的每個級別，是否個別行太復(fù)雜？功能是否太復(fù)雜？類是否太復(fù)雜？”復(fù)雜“意味著代碼閱讀者很難快速理解代碼，也可意味著開發(fā)者嘗試調(diào)用或修改此代碼時可能會引入bug。
一個典型的復(fù)雜性問題就是 過度設(shè)計，當(dāng)開發(fā)者讓代碼變得更通用，或者增加了許多當(dāng)前不需要的功能時，評審者應(yīng)該額外注意是否過度設(shè)計。鼓勵開發(fā)者解決現(xiàn)在遇到的問題，而不是揣測未來可能遇到的問題。未來的問題應(yīng)當(dāng)在遇到時解決，到那個時候你才能看到問題本質(zhì)和實際需求。
測試
開發(fā)人員應(yīng)當(dāng)進(jìn)行單元測試、集成測試或端到端測試。一般來說，變更中應(yīng)該包含測試，除非這個變更只是為了處理緊急情況。
確保變更中的測試是正確、合理和有用的。因為測試本身無法測試自己，而且我們很少會為測試編寫測試，所以必須確保測試是有效的。
如果代碼出了問題，測試會失敗嗎？如果代碼發(fā)生改動，它們會誤報嗎？每一個測試都有斷言嗎？是否按照不同的測試方法對測試進(jìn)行分類？
記住，不要以為測試不是二進(jìn)制中的一部分就不關(guān)注其復(fù)雜度。
命名
開發(fā)人員是否使用了良好的命名方式？好的命名要能夠充分表達(dá)一個項（變量、類名等）是什么或者用來做什么，但又不至于讓人難以閱讀。
注釋
開發(fā)者有沒有寫出清晰易懂的注釋？所有的注釋都是必要的嗎？通常注釋應(yīng)該解釋清楚為什么這么做，而不是做了什么。如果代碼不清晰，不能清楚地解釋自己，那么代碼可以寫的更簡單。當(dāng)然也有些例外（比如正則表達(dá)式和復(fù)雜的算法，如果能夠解釋清楚他們在做什么，肯定會讓閱讀代碼的人受益良多），但大多數(shù)注釋應(yīng)該指代碼中沒有包含的信息，比如代碼背后的決策。
變更中附帶的其他注釋也很重要，比如列出一個可以移除的待辦事項，或者一個不要做出代碼變更的建議，等等。
注意，注釋不同于類、模塊或函數(shù)文檔，文檔是為了說明代碼片段如何使用和使用時代碼的行為。
風(fēng)格
在谷歌內(nèi)部，主流語言甚至部分非主流語言都有編碼風(fēng)格指南 ，確保變更遵循了這些風(fēng)格規(guī)范。
如果你想對風(fēng)格指南中沒有提及的風(fēng)格做出改進(jìn)，可以在注釋前面加上“Nit:”，讓開發(fā)人員知道這是一個你認(rèn)為可以改進(jìn)的地方，但不是強(qiáng)制性的。但請不要只是基于個人偏好來阻止變更的提交。
開發(fā)人員不應(yīng)該將風(fēng)格變更與其他變更放在一起，這樣很難看出變更里有哪些變化，讓合并和回滾變得更加復(fù)雜，也會導(dǎo)致更多的其他問題。如果開發(fā)者想要重新格式化整個文件，讓他們將重新格式化后的文件作為單獨的變更，并將功能變更作為另一個變更。
文檔
如果變更改變了用戶構(gòu)建、測試、交互或者發(fā)布代碼相關(guān)的邏輯，檢測是否也更新了相關(guān)文檔，包括READMEs, g3doc頁面，以及任何相關(guān)文檔。如果開發(fā)者刪除或者棄用某些代碼，考慮是否也應(yīng)該刪除相關(guān)文檔。如果文檔有確實，讓開發(fā)者補(bǔ)充。
代碼細(xì)節(jié)
查看你整個Code Review中的每一行代碼，比如你可以掃到的數(shù)據(jù)文件，生成的代碼或大型數(shù)據(jù)結(jié)構(gòu)，但不要只掃一遍手寫的類，函數(shù)或者代碼塊，然后假設(shè)它們都能正常運行。顯然，有些代碼需要仔細(xì)檢查，這完全取決于你，但你至少應(yīng)該理解所有的代碼都在做什么。
如果你很難看懂代碼，導(dǎo)致Code Review的速度慢了下來，你要讓開發(fā)者知道，并且在Review前澄清原因。在谷歌，我們有最優(yōu)秀的工程師，你也是其中之一。如果你都看不懂，很可能其他人也看不懂。所以要求開發(fā)者理清代碼邏輯也是在幫助未來的開發(fā)者。
如果你理解代碼，但又覺得沒有資格做代碼評審，確保有資格的變更評審人員在代碼評審時考慮到了安全性、并發(fā)性、可訪問性、國際化等問題。
上下文
看改動上下文代碼對Code Review很有幫助，因為通常Code Review工具只會顯示改動部分上下幾行代碼，但有時你必須查看整個文件確保這次變更可以正常運行。例如，你可能看到加了4行代碼，但是看到整個文件時才會發(fā)現(xiàn)這加的4行代碼是在一個50多行的方法里，這個方法現(xiàn)在應(yīng)該被拆分為多個小的方法了。
Code Review時考慮到整個系統(tǒng)的上下文也很重要，這次改動提升了系統(tǒng)健康度？或者增加了系統(tǒng)復(fù)雜性？少了測試用例？…… 不要通過哪些會損害系統(tǒng)健康的代碼。很多系統(tǒng)變復(fù)雜都是因為一點一點的小改動日積月累造成的，所以千萬不要放進(jìn)來任何增加復(fù)雜性的改動。
亮點
如果你看到變更中做的好的地方，告訴開發(fā)者他們做的很棒，尤其是當(dāng)他們用某種很精巧的方式解決你評論中提到的問題時更應(yīng)不吝贊美。Code Review過多關(guān)注于錯誤，但你也應(yīng)該為開發(fā)者展現(xiàn)出好的一面點贊。在指導(dǎo)他人的時候，有時候告訴開發(fā)者正確的做法比告訴他們錯誤的做法更有價值。
總結(jié)
在做Code Review時，你需要注意以下：
-
代碼設(shè)計良好。
-
代碼功能對用戶有用。
-
任何UI改動應(yīng)當(dāng)是深思熟慮過而且看起來不錯的。
-
保證線程安全。
-
代碼沒有增加不必要的復(fù)雜性。
-
開發(fā)者沒有寫有些將來需要但現(xiàn)在不知道是否需要的東西。
-
代碼有適當(dāng)?shù)膯卧獪y試。
-
測試邏輯設(shè)計良好。
-
開發(fā)者使用了清晰明了的命名。
-
注釋清晰明了實用，通常解釋清楚了為什么這么做，而不是做了啥。
-
代碼又相應(yīng)完善的文檔。
-
代碼風(fēng)格符合規(guī)范。
確保你review了要求你看的每一行代碼，確保你正在提升代碼質(zhì)量，并且為開發(fā)者做的提升點贊。
Code Review步驟
概要
現(xiàn)在你知道了要從CL中得到什么，那能在眾多文件中完成評審的方法是什么？
-
這條改動是否生效？這次變更是不是描述清晰?
-
首先閱讀CL最重要的一部分，整體上是否設(shè)計良好？
-
按照合適的順序閱讀剩下的變更。
第一步: 綜觀這個改動
閱讀CL描述并且明白CL大體內(nèi)容。這些修改是否有意義？首先如果這個修改不應(yīng)該有，請立刻說明為什么這些修改不應(yīng)該有。當(dāng)你因為這個拒絕了這次改動提交時，告訴開發(fā)人員該怎么去做是非常好的。
例如，您可能會說：“看起來您為此做了一些出色的工作，謝謝！但是，我們實際上正著手刪除FooWidget系統(tǒng)，您正在此處進(jìn)行修改，因此我們不想進(jìn)行任何新的修改現(xiàn)在。所以，您可以重構(gòu)我們的新BarWidget類嗎？"
請注意，審閱者不僅拒絕了當(dāng)前的CL并提供了替代建議，但他們禮貌地做到了。這種禮貌是重要，因為我們想表明我們甚至在開發(fā)人員之間也相互尊重，尤其是當(dāng)我們意見不同時。
如果您得到的多個CL并且您不想進(jìn)行的更改，您應(yīng)該考慮重新設(shè)計團(tuán)隊的開發(fā)流程或發(fā)布的外部貢獻(xiàn)者的流程，以便在CL完成之前進(jìn)行更多的交流。最好在做大量工作之前告訴人們“不必做”，因為現(xiàn)在要將其丟棄或徹底重寫。
第二步: 檢查CL的主要部分
查找屬于此CL的“主要”部分的文件。通常來說一個CL最重要的部分是它邏輯修改數(shù)最多的那個文件。這樣有助于了解相關(guān)的CL，通常來說會加快code review。如果CL太大，您無法確定哪些部分是主要部分，請開發(fā)人員告訴你應(yīng)該首先看什么，或要求他們將CL拆分為多個CL。
如果您發(fā)現(xiàn)CL的這一部分存在一些主要的設(shè)計問題，則即使您現(xiàn)在沒有時間審查CL的其余部分，也應(yīng)立即寫下這些評注。實際上，檢查其余的CL可能會浪費時間，因為如果設(shè)計問題足夠嚴(yán)重，那么許多其他正在檢查的代碼將消失并且無論如何都不會發(fā)生。
立即寫下這些主要設(shè)計評注非常重要有兩個主要原因：
-
開發(fā)人員通常會發(fā)送給審核者一個CL，然后在等待審核時立即基于該CL進(jìn)行新工作。如果您正在審查的CL中存在重大設(shè)計問題，那么他們也將不得不重新設(shè)計其以后的CL。你能在他們做太多無用功之前制止他們。
-
重要的設(shè)計變更比小的變更需要更長的時間。開發(fā)人員幾乎都有截止日期；為了在截止日期之前完成工作，并在代碼庫中保留高質(zhì)量的代碼，開發(fā)人員需要盡快開始CL的所有主要的重做。
第三步:依序閱讀CL的其余部分
確認(rèn)CL整體上沒有大的設(shè)計問題后，請嘗試找出邏輯順序來瀏覽文件，同時還要確保不要錯過對任何文件的審查。通常，在瀏覽了主要文件之后，按照代碼審查工具向您展示它們的順序瀏覽每個文件是最簡單的。有時在閱讀主要代碼之前先閱讀測試代碼也是有幫助的，因為這樣您就可以知道更改應(yīng)該做什么。
Code Review的速度
為什么Code Review應(yīng)該快？
在谷歌內(nèi)部，我們在持續(xù)優(yōu)化團(tuán)隊開發(fā)新產(chǎn)品的速度，而不是優(yōu)化單個開發(fā)人員編寫代碼的速度。個人開發(fā)的速度固然重要，但它沒有團(tuán)隊整體的速度那么重要。
如果Code Review速度慢了，可能會發(fā)生以下這些事：
-
整個團(tuán)隊的速度會降低，是的，你不快速響應(yīng)別人的Code Review也可以完成其他的工作。但是，整個團(tuán)隊的新功能、bug修復(fù)可能會因為沒有人做cr被延遲幾天、幾周甚至幾個月。
-
開發(fā)者應(yīng)維護(hù)Code Review的流程，如果審查者很少回復(fù)Code Review，但是每次都對CL提出大量的改動，這可能會打擊到開發(fā)者。通常，開發(fā)者可能會抱怨審查者太嚴(yán)格。如果審查者能在開發(fā)者更新后快速響應(yīng)，并提出有實質(zhì)性提升的建議（能顯著提升代碼運行狀況的CL），抱怨就會消失。Code Review中絕大多數(shù)抱怨會隨著CR速度的提升而消失。
-
可能影響到代碼質(zhì)量。如果CR慢了，可能會給開發(fā)者一種需要提交不太好代碼的壓力。CR慢了，也會影響到代碼清理、重構(gòu)、和現(xiàn)有CL的進(jìn)一步提升。
應(yīng)該以什么樣的速度做Code Review？
如果你不是在做需要高度專注的任務(wù)，Code Review應(yīng)該越快越好。應(yīng)當(dāng)在一個工作日之內(nèi)回應(yīng)Code Review請求。遵循這些指導(dǎo)意味著典型的CL應(yīng)該在一天內(nèi)進(jìn)行多輪審查（如果需要）。
速度 vs 中斷
只有一種情況下，個人的速度要勝過團(tuán)隊速度。如果您正在處理諸如編寫代碼之類的重要工作，請不要打斷自己的工作去做Code Review，研究人在專注中被打斷后需要很長的時間才能重新恢復(fù)到專注狀態(tài)。所以，為了不讓其他開發(fā)者等會而且中斷自己的編碼工作，明顯得不償失。
所以，建議你在工作斷點的時候回應(yīng)Code Review，比如寫完代碼、午飯后、會議結(jié)束后、從茶水間回來……
快速響應(yīng)
當(dāng)我們談?wù)揅ode Review的速度時，一方面是指響應(yīng)時間，另一方面是指整個Review從提交到通過的時間。整個過程應(yīng)該足夠快，但是對于每個人來說，迅速做出反應(yīng)比迅速完成整個過程更為重要。
即使有時需要很長時間才能完成整個Code Review流程，評審者在整個過程中的快速響應(yīng)也可以大大緩解開發(fā)人員因為Code Review“慢”而產(chǎn)生的挫敗感。
如果你太忙不能全身心投入到Code Review中，你也應(yīng)該讓開發(fā)者知道你什么時候會去Review，或者建議其他評審者快速響應(yīng)，再或者提供一些初步的建議。（注意：這不是建議你中斷自己的工作，而是在工作間隙合理響應(yīng)）
評審者有必要花時間去做Code Review來保證代碼符合標(biāo)準(zhǔn)，不管怎么樣，每個回應(yīng)應(yīng)當(dāng)保證足夠快速。
跨時區(qū)Code Review
當(dāng)遇到跨時區(qū)的Code Review時，盡量在作者回家前回復(fù)。如果他們已經(jīng)回家了，盡量在他們每天來公司前完成Code Review。
LGTM和注釋
為了讓CR更快，有些情況下也應(yīng)該讓CR提前通過，即便有些評論沒有被解決，比如：
-
審查者信任開發(fā)者能適當(dāng)解決所有評審者的建議。
-
其余的改動很小，開發(fā)者不必做。
除非另有明確說明，評審者應(yīng)指明他們打算使用這些選項中的哪一個。
當(dāng)開發(fā)者和評審者在不同時區(qū)時，應(yīng)當(dāng)著重考慮下通過CR，否則開發(fā)者還得等一天。
大的CL
如果有人給你提交了一個非常大的Code Review，你也不確定你有時間看，你最好建議開發(fā)者把CL拆分成幾個小的部分，分多次Code Review，而不是一次性全部提交上來。這即便開發(fā)者多做一些額外的工作，也是可以把它拆分開的，而且拆分也有利于評審者。
如果CL不能拆分成多個小CL，你也沒有時間快速完整的Review整個代碼，只是對整體設(shè)計提一些建議，然后發(fā)回給開發(fā)者改進(jìn)。作為評審者，你的目標(biāo)之一是在不犧牲代碼質(zhì)量的前提下，不阻礙開發(fā)者的進(jìn)程或者盡可能讓他們向前推進(jìn)。
持續(xù)提升Code Review
如果你遵從這些建議并在Code Review中嚴(yán)格執(zhí)行這些準(zhǔn)則，你就會發(fā)現(xiàn)整個Code Review的流程會越來越快。開發(fā)者將了解健康代碼的要求，并從一開始就交出完美的代碼，然后Code Review的時間會越來越少。評審者將學(xué)會如何快速做出響應(yīng)，并且不是在這個Code Review過程中增加不必要的延遲。
但是，不要為了提升速度犧牲Code Review的標(biāo)準(zhǔn)和代碼質(zhì)量。從長遠(yuǎn)來看，這并不會提升速度。
緊急情況
當(dāng)然也有一些緊急的CL需要快速走完這個Code Review流程，這時候在質(zhì)量上的把控可以稍微放松一些。可以參考緊急事件一文來了解哪些是緊急事件哪些不是。
怎樣寫代碼評論
概要
-
禮貌
-
解釋你的觀點.
-
明確指出方向和問題，幫助開發(fā)人員去權(quán)衡作出決定.
-
鼓勵開發(fā)人員通過注釋和精簡代碼來解決你的困惑而不是通過解釋
禮貌
通常來說當(dāng)你Code Review代碼時保持禮貌和尊重能使開發(fā)人員更加清晰，得到更多幫助。這樣是為了保證你的代碼評論僅僅針對的是code而不是針對開發(fā)人員。你不必一直這么去做，但是當(dāng)你的評論會讓開發(fā)人員生氣或者產(chǎn)生爭執(zhí)時有必要這么去做。比如：
不好的例子: “你為什么會在這里使用線程，這樣做難道會有任何好處？”
好的例子: "我并沒有發(fā)現(xiàn)這個并發(fā)模塊給程序帶來了多少幫助，并且還增加了程序的復(fù)雜性，因此我認(rèn)為這段代碼最好是用單線程而不是多線程。
解釋清楚原因
從上面“好”的例子當(dāng)中你能發(fā)現(xiàn)，這樣有助于開發(fā)人員理解為什么你寫了這些評注。你不一定非得包含這些信息在你的評注里面，但是適當(dāng)?shù)亩嘟忉屇愕囊鈭D或者多給出一些提升代碼質(zhì)量的建議都是非常好的實踐。
給予指導(dǎo)
通常來說修復(fù)CL是開發(fā)人員的職責(zé)而不是評審人員的。你不需要向開發(fā)人員提供詳細(xì)的解決方案或者代碼。
但是這并不意味著評審員就不應(yīng)該提供幫助。你需要在指出問題和提供直接指導(dǎo)之間找到平衡。指出問題并且?guī)椭_發(fā)人員決策能夠幫助開發(fā)人員學(xué)同事讓code review變得更加簡單。這樣也能產(chǎn)生更好的方案，因為開發(fā)人員比評審者更加了解代碼。
盡管這樣，有時候直接給出指導(dǎo)，建議甚至是代碼更有幫助。code review的主要目的是盡可能得到最好的CL。其次是提高開發(fā)人員的技能這樣就能減少以后評審的次數(shù)。
接受解釋
與其要求讓開發(fā)人員解釋一段你看不懂的代碼，其實更應(yīng)該做的是讓他們重寫代碼，讓代碼更清晰。當(dāng)一段代碼不是太過于復(fù)雜的時候通過加一些注釋偶爾也是一種不錯的做法。
解釋僅僅是寫在Code Review工具里的，不利于將來的代碼閱讀者。只有極少數(shù)情況是可行的，比如你對你評審的需求不太熟悉，但是開發(fā)人員解釋的是大多數(shù)人都知道的。
處理Code Review中的反駁
有時開發(fā)者會在Code Review中反駁你，他們可能不同意你的意見，或者抱怨你太嚴(yán)格了。
誰是誰非？
當(dāng)開發(fā)者不同意你的建議時，首先花點思考下他們是否是對的，但通常而言你比他們更熟悉代碼，所以可能在某個方面理解更深。他們的爭論有意義嗎？從代碼健康的角度來看他們的反駁有意義嗎？如果是，讓他們知道他們是對的，然后這個問題就解決了。
然而，開發(fā)者不總是對的，這種情況下評審者應(yīng)當(dāng)進(jìn)一步介紹為什么他們的建議是對的。好的解釋不僅得體現(xiàn)出對開發(fā)人員回復(fù)的理解，而且還要說明為什么要這么改。
如果評審者認(rèn)為他們的建議可以改善代碼質(zhì)量，并且他們認(rèn)為帶來的代碼質(zhì)量改進(jìn)值得開發(fā)者做這些額外的工作，評審者就應(yīng)該堅持自己的立場。
不積跬步無以至千里，不積小流無以成江河
有時候要讓開發(fā)者接受，你得花很多時間反復(fù)解釋，但始終確保該有的禮貌，并讓開發(fā)者知道在知道他們了什么，即便是你不同意。
惹惱開發(fā)者
有時評審者會認(rèn)為堅持讓開發(fā)者做出改動，可能會惹惱開發(fā)者。有時開發(fā)者確實會惱怒，但這種情況通常都很短暫，而且之后他們會感激你幫助他們提高了代碼質(zhì)量。如果你在Code Review中很有禮貌，開發(fā)者根本不會被惹惱，這種擔(dān)心是多余的。通常，令惹惱開發(fā)者的是你寫注解的方式，而不是你對代碼質(zhì)量的堅持。
稍后解決
一種常見的反駁原因是開發(fā)者希望能盡快完成任務(wù)。他們不想一輪又一輪地做Code Review，然后就會說他們會在后續(xù)CL中處理這些問題，你只需要通過就行。有些開發(fā)者做的很好，他們會立馬提交后續(xù)CL處理這些問題。然而，經(jīng)驗告訴我們原始CL通過之后拖的時間越久，就越不可能修復(fù)。除非開發(fā)者在當(dāng)前CL通過后立馬修復(fù)，否則他們就不可能修復(fù)。這并不是開發(fā)者不負(fù)責(zé)任，而是因為他們有好多工作要做，而修復(fù)工作也會因為工作壓力而被遺忘。所以最好堅持讓開發(fā)者現(xiàn)在就在CL中處理掉這些問題，“留著以后清理”是一種不可取的方式。
如果CL中引入了新的復(fù)雜性，提交之前必須清理掉，除非是緊急情況。如果CL中暴露出一些目前還無法定位的問題，開發(fā)者應(yīng)該記錄下這些bug，并將其分配給他們自己，確保這些問題不會被遺忘。他們還可以在代碼中加入 TODO 注釋，指向已經(jīng)記錄好的 bug。
抱怨太嚴(yán)格
如果你之前Code Review很寬容，然后突然變得嚴(yán)格起來，可能會引起一些開發(fā)者的抱怨。不過沒關(guān)系，加快Code Review的速度通常會讓這些抱怨消失。
有時，這些抱怨可能需要幾個月的時間才能消除，但最終開發(fā)者在看到產(chǎn)出的優(yōu)質(zhì)代碼時會理解嚴(yán)格Code Review帶來的價值。有時候，一旦發(fā)生某些事讓他們真正看到嚴(yán)格Code Review的價值，抗議最大聲的人甚至?xí)蔀槟阕顖远ǖ闹С终摺?/p>
解決沖突
如果你遵循了上述方法，但仍然會在Code Review中遇到無法解決的沖突，請再次參閱Code Review標(biāo)準(zhǔn)，了解那些有助于解決沖突的指導(dǎo)和原則。
3.發(fā)者指南
寫一個好的CL描述
一個CL描述是做了什么更改以及為什么的公開記錄。
它將成為我們版本控制歷史的永久組成部分，多年來除你的評審者以外，還可能有數(shù)百人會閱讀它。
將來的開發(fā)者將會基于它的描述來搜索你的CL。
將來可能會有人由于沒有現(xiàn)成的細(xì)節(jié)，而根據(jù)他模糊的記憶來尋找你的改變。
如果所有重要的細(xì)節(jié)都在代碼中而不是描述中，那么他們將很難定位你的CL。
第一行
-
言簡意賅
-
語義完整
-
空行隔開
CL描述的第一行應(yīng)該是對CL正在做的具體工作的簡短總結(jié)，緊跟一個空行。在未來，這是大多數(shù)的代碼搜索者在瀏覽一段代碼的版本控制歷史時會看到的，因此第一行應(yīng)該足夠有信息量，他們不必閱讀你的CL或它的整個描述就可以大致了解你的CL實際做了什么。
按照傳統(tǒng)，CL描述的第一行是一個完整的句子，就像它是一個命令（一個祈使句）。例如，說"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)容。它可能包括對正在解決的問題的簡要描述，以及為什么這是最好的方法。如果這種方法有任何缺點，就應(yīng)該提到。將相關(guān)的背景信息（例如bug數(shù)目，基準(zhǔn)測試結(jié)果），以及設(shè)計文檔的鏈接。即使是很小的CL也值得關(guān)注細(xì)節(jié)。請將來龍去脈放入CL中。
反例
"Fix bug"是一個不充分的CL描述。是什么bug？你是怎么修復(fù)它的？
其他一些類似的不好的CL描述:
-
“Fix build.”
-
“Add patch.”
-
“Moving code from A to B.”
-
“Phase 1.”
-
“Add convenience functions.”
-
“kill weird URLs.”
這里有一些是真實的CL描述。他們的作者可能認(rèn)為他們提供的信息是有用的，但他們并沒有給出一個CL描述的意圖。
好的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.
前幾個詞描述了CL描述實際做了什么。其余描述在說明解決的問題，為什么這是一個好的方案，以及具體實現(xiàn)的細(xì)節(jié)。
重構(gòu)
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ā)生變化的。
其余的描述將討論具體的實現(xiàn)、CL的來龍去脈、解決方案的不理想以及未來可能的方向。它同樣是解釋為什么要進(jìn)行這個修改。
需要上下文的小CL
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.
第一句描述了實際做了什么。其余的描述解釋了為什么要進(jìn)行更改，并給了reviewer很多背景。
在提交CL之前，請檢查描述
在Review期間CL可能會經(jīng)歷重大改變。在提交CL之前，有必要review CL描述，以確保描述仍然反映CL所做的事情。
簡短化的CL
為什么要寫成一系列簡短的CL?
簡短的CL有這些好處:
-
Code Review更快 與比起花30分鐘審查一個大型的CL相比，對審查者來說花5分鐘審查一系列小的CL更加容易.
-
審查更加徹底。進(jìn)行較大的更改后，審閱者和作者往往會因大量詳細(xì)評論的來回移動而感到沮喪，有時這些評論會漏掉或遺漏重要的觀點。
-
減少導(dǎo)致bug的可能性。由于您所做的更改較少，因此您和您的審閱者更容易有效地推斷出CL的影響，并查看是否導(dǎo)致bug。
-
減少不必要的工作 當(dāng)你寫了一個巨大的CL，然后審查者覺得你總體方向錯了，這會導(dǎo)致你浪費大量的工作。
-
更方便合并代碼 因為大型的CL會導(dǎo)致大量的沖突，因此合并大型的CL會浪費很多時間，并且這將會是你經(jīng)常做的工作。
-
有助于你作出更好的設(shè)計 優(yōu)雅的設(shè)計并且完善一個小的改動比大的改動更加容易
-
降低審查者的難度 提審部分改動，不會影響你繼續(xù)編碼。
-
回滾更容易 大型CL很有可能會在初始CL提交和回滾CL之間更新修改文件，從而使回滾變得復(fù)雜（中型CL也可能也需要回滾可能也會這樣）。
注意！審查者可以因為你的改動過于巨大直接拒絕掉你通常，他們會感謝您的貢獻(xiàn)，但要求您以某種方式使它成為一系列較小的更改。不管是你把這些改動拆分成小的改動，還是和審查者爭論讓他接受都會耗費你大量的時間。但是編寫小型改動就不會有這樣的問題。
怎么樣算簡短?
通常，CL的正確大小是一個獨立的更改。這意味著：
-
CL所做的最小更改僅解決了一件事情。通常，這只是功能的一部分，而不是一次完整的功能。通常，寧可編寫過小的CL也不要寫太大的CL。你可以和你的審查者商量找到合適的尺度。
-
審閱者需要了解的有關(guān)CL的所有內(nèi)容（將來的開發(fā)除外）都在CL，CL的描述，現(xiàn)有代碼庫或他們已經(jīng)查看過的CL中。
-
檢入CL后，系統(tǒng)將繼續(xù)對其用戶和開發(fā)人員正常運行。
-
CL不夠小的話會導(dǎo)致其難以理解。如果你新增加了api，那么在同一個CL應(yīng)該包括這個api用到的方法，以便審查者更好的理解。也能防止檢入沒有用的api。
多大算大，沒有一個明確的要求。對于CL而言，100行通常是一個合理的大小，而1000行通常太大，但這取決于您的審閱者的判斷。更改分布的文件數(shù)量也會影響其“大小”。可以在一個文件中進(jìn)行200行的更改，但是將其分布在50個文件中通常會太大。
請記住，盡管從您開始編寫代碼起就一直與您緊密聯(lián)系，但審閱者通常沒有上下文。對您來說，看起來像可接受大小的CL可能會讓您的審閱者不知所措。毫無疑問，盡可能把CL些小是正確的。審查者很少抱怨CL太小
什么時候大型的CL也是可以的?
在某些情況下，較大的更改沒有那么糟糕:
-
通常，您可以將整個文件的刪除視為更改的一行，因為它不會花費很長時間來審閱審閱者。
-
有時，您完全信任的自動重構(gòu)工具已經(jīng)生成了一個較大的CL，而審閱者的工作只是檢查健全性，然后指出他們認(rèn)為需要修改的地方。這些CL可能更大，盡管會有一些風(fēng)險（例如合并和測試）仍然適用。
按文件分類
拆分CL的另一種方法是通過將文件分組，如果這些文件將是獨立的更改，可以分配各不同的審閱者。
比如: 你提交一個修改的CL，創(chuàng)建一個修改的緩沖區(qū)，另一個CL的代碼修改也可以提交到這里面。你必須按照順序提交CL，但是審閱者可以同時進(jìn)行審閱. 如果這么做，你需要盡可能告訴所有審閱者另一個CL的信息，以便他們能得到上線文信息。
另一個示例：您發(fā)送一個CL進(jìn)行代碼更改，而另一個CL則發(fā)送使用該代碼的配置或?qū)嶒?；如果需要，這也更容易回滾，因為有時將配置/實驗文件推送到生產(chǎn)環(huán)境中比更改代碼更快。
把代碼重構(gòu)分離出來
通常重構(gòu)最好不要和功能修改或者bug fix一起提CL。比如移動或者重命名一個Class，最好和這個Class的bug fix分開提CL。這樣對于審閱者來說，理解每個CL單獨引入的更改要容易得多。
不過一些小的代碼清理工作比如變量重命名可以包含在功能修改或者bug fix的CL種。這取決于開發(fā)者和審閱者的判斷，當(dāng)然如果巨大的重構(gòu)包含在同一個CL中會大大增加審閱的難度。
將相關(guān)的測試代碼保存在同一CL中
避免將測試代碼拆分為單獨的CL。驗證代碼修改的測試應(yīng)該進(jìn)入相同的CL，即使這樣做會增加代碼行數(shù)。
但是根據(jù)重構(gòu)準(zhǔn)則，獨立的測試修改可以單獨寫入CL。比如
-
使用新測試驗證先前存在的提交代碼。
-
重構(gòu)測試代碼（例如，引入輔助函數(shù)）。
-
引入更大的測試框架代碼（例如集成測試）。
不要破壞結(jié)構(gòu)
如果您有多個相互依賴的CL，則需要找到一種方法來確保在提交每個CL之后整個系統(tǒng)都能正常工作。否則，可能破壞代碼結(jié)構(gòu)導(dǎo)致后面的開發(fā)者浪費大量的時間等你的提交（如果這些CL提交出了問題，則更長的時間）。
小到不能再小
有時候，您會遇到CL很大的情況。這很少是真的。練習(xí)編寫小型CL的作者幾乎總是可以找到一種將功能分解為一系列小的更改的方法。
在寫大型CL之前，思考下是不是能夠?qū)⒅貥?gòu)分離出來，這是一個更加清晰的思路。多和你的隊友交流學(xué)習(xí)下他們對縮小CL的實踐和方法。
如果所有這些選項都失敗了（這種情況很少見），那么請事先獲得您的審閱者的同意，告訴他們一個巨大的CL將要來臨。在這種情況下，審查過程會耗費極其長的實踐，這樣請花費更多的時間去寫測試代碼，避免引入bug。
如何處理評審者的評論
當(dāng)你提交了一個變更做Code Review時，很可能你都會收到評審者在變更中的評論。這里有一些處理這些評論的建議。
不要摻雜個人情感
Code Review的目標(biāo)是維護(hù)代碼庫和產(chǎn)品的質(zhì)量。如果評審者批評了你的代碼，可以理解為他們在幫你、幫整個代碼庫、甚至是幫整個公司，而不是攻擊你或者是質(zhì)疑你的能力。
有時候評審者會情緒低落，然后在評論中說出一些令人沮喪的話，雖然評審者這樣做不對，但作為開發(fā)者你應(yīng)當(dāng)有心理準(zhǔn)備。問問你自己，“評審者試圖與我交流的建設(shè)性意見是什么？” 然后照他們說的那些去做。
永遠(yuǎn)不要回應(yīng)充滿怒氣的評論，Code Review工具中違反職業(yè)禮儀的情況永遠(yuǎn)存在。如果你真的忍無可忍，建議先離開電腦也會兒，或者干一些其他的事，等心情平復(fù)下來再回復(fù)。
通常，如果評審者沒有以禮貌的方式提供反饋，請親自向他們解釋。如果你無法親自或者通過視頻和他們聯(lián)系，就向他們發(fā)一份私人郵件。友好地告訴他們你不喜歡的事情以及你希望他們做些什么。如果他們也以非建設(shè)性的方式對此私密討論做出回應(yīng)，或者沒有起到預(yù)期的作用，請酌情上報給您的經(jīng)理。如果他們已經(jīng)以不禮貌的方式回應(yīng)，或者沒有取得預(yù)期的效果，視情況匯報給你的經(jīng)理。
修復(fù)代碼
如果評審者說他們理解不了你代碼中的某些內(nèi)容，你首先應(yīng)該把代碼寫清晰。如果讓代碼更清晰，添加注釋來解釋清楚代碼的邏輯。如果評論似乎毫無意義，那么您的答復(fù)應(yīng)該只是代碼查看工具中的解釋。
如果評審者無法理解你的某部分代碼，那邊可能未來的閱讀者也可能理解不了。在Code Review工具中回應(yīng)幫不了未來的讀者，但是代碼中的注釋可以。
自我反思
寫一個變更會花費你很大的精力，提價Code Review時會感覺如釋負(fù)重，而且自己也相當(dāng)確定所有工作已經(jīng)做完了。所以當(dāng)評審者提出改進(jìn)建議時，你很容易認(rèn)為那些都是錯的，或者認(rèn)為是評審者給你不必要的阻撓，再或者覺得評審者應(yīng)該讓你提價變更。無論如何，不管你怎么想，花點時間回想下評審者給你的反饋有助于提升公司的代碼質(zhì)量。你始終問下自己“如果評審者是對的呢？”
如果你回答不了評審者的問題，那可能說明評審者的評論不夠清楚。
如果你認(rèn)真考慮過后依舊認(rèn)為你是對的，放心大膽地解釋清楚為什么你的方法對公司更有利。通常，評審者只是提供建議，并且希望你能思考出更好的方法。也許你已經(jīng)知道一些評審者不知道的關(guān)于用戶、代碼庫、或者變更，把這些都寫下來，給評審者更多的上下文信息，通常你都可以根據(jù)某些事實和評審者達(dá)成某些共識。
解決沖突
解決沖突的第一步，和你的評審者達(dá)成共識，如果無法達(dá)成共識，參閱Code Review的標(biāo)準(zhǔn)獲取更多內(nèi)容。
版權(quán)聲明：本文為CSDN博主「帥昕 xindoo」翻譯，版權(quán)歸作者所有。
