谷歌開源内部代碼評審規範

代碼評審标準

代碼評審的主要目的是確定代碼庫的整體品質随時間推移逐漸得到提升，所有代碼評審工具和過程都是為了實作這一目标而設計的。

為了實作這個目标，必須做出一系列權衡。首先，開發人員的開發任務必須要有所進展。如果他們不送出改進的代碼，代碼庫品質就得不到改善。此外，如果評審人員過于嚴格，開發人員就沒有動力進行持續改進。

評審人員的職責是確定每個 CL（變更清單）的品質，保證代碼庫整體品質不會随着時間的推移而下降。這是一項艱巨的任務，因為代碼庫整體品質常常會随着每次送出代碼品質的小幅下降而退化，特别是有時候開發團隊時間很緊，并認為必須走捷徑才能完成傳遞任務。

評審人員要對他們評審的代碼負起責任，確定代碼庫保持一緻性和可維護性。

以下是可在代碼評審中使用的準則：

一般來說，如果 CL 達到可以提升系統整體代碼品質的程度，就可以讓它們通過了，即使它們可能還不完美。

這是所有代碼評審準則的最高原則。

當然，也有例外的時候。例如，如果 CL 中包含了系統不需要的功能，那麼即使代碼寫得很好，評審人員也可以拒絕讓它們通過。

這個世界上沒有“完美”的代碼，隻有更好的代碼。評審人員不應該要求開發人員對 CL 中的每一個微小部分都進行細緻入微的打磨，而應該在滿足需求和變更重要性之間做出權衡。評審人員不應該追求完美，而應該追求持續改進。如果一個 CL 能夠從整體上提高系統的可維護性、可讀性和可了解性，那它就不應該僅僅因為它不夠“完美”而被延遲幾天甚至幾周。

評審人員應該提供建議，告訴開發人員哪些方面可以做得更好。但如果這些建議不是很重要，可以在前面加上像“Nit:”這樣的字首，讓開發人員知道這隻是一個改進建議，他們也可以選擇忽略。

指導

代碼評審的一個作用是向開發人員傳授知識，比如關于一門語言、一個架構或一般軟體設計原則的知識。分享知識是提升系統代碼品質的一個組成部分。但要注意，如果你的建議純粹是帶有教育性質的，并且對于滿足本文所描述的标準來說并不是那麼重要，那麼請在前面加上“Nit:”，或者以其他方式告訴開發人員，他們并不一定要在 CL 中解決這些問題。

原則

• 客觀的技術和資料比個人意見和偏好更重要。
• 在代碼風格方面，可以參考谷歌風格指南（ http://google.github.io/styleguide ）。任何沒有在這個風格指南中出現的東西（比如空格等）都屬于個人偏好。代碼風格應該與原有代碼保持一緻，如果之前沒有規定代碼風格，可以使用代碼送出者的代碼風格。
• 軟體設計從來就不隻風格問題，也不隻是個人偏好問題。它們建立在一些基本原則之上，是以我們應該基于這些原則做出權衡，而不隻是基于個人偏好。有時候，一個問題有多種解決方案，如果開發人員能夠證明（通過資料或基于可靠的工程原理）幾種解決方案是同樣有效的，那麼評審人員應該接受開發人員的選擇，否則就應該基于軟體設計标準原則做出決定。
• 如果沒有其他适用的原則，評審人員可以要求開發人員與目前代碼庫保持一緻，隻要不破壞系統的整體代碼品質。

解決沖突

在代碼評審過程中出現沖突時，開發人員和評審人員首先要嘗試根據本文、CL 作者指南（

https://google.github.io/eng-practices/review/developer/

）和評審人員指南（

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

）達成一緻意見。

如果很難達成一緻意見，評審人員和開發人員可以進行面對面會議或者視訊會議，而不是隻是試圖通過代碼評審評論闆來解決沖突。

如果還不能解決問題，那麼就要考慮把問題更新，進行更廣泛的團隊讨論。讓團隊負責人參與進來，請求代碼維護人員作出決定，或請求工程經理提供幫助。不要因為開發人員和評審人員無法達成一緻意見就讓 CL 一直挂在那裡。

代碼評審要注意哪些事情？

設計

代碼評審中最重要的部分是 CL 的總體設計。CL 中不同代碼段之間的互動是有意義的嗎？這個變更應該屬于代碼庫，還是屬于某個包？它與系統的其他部分可以良好地內建嗎？現在是引入這個變更的好時機嗎？

功能

這個 CL 是否達到了開發人員的目的？開發人員的意圖對代碼使用者來說有好處嗎？代碼“使用者”可以是指最終使用者（他們受代碼變更的影響）和開發人員（将來要“使用”這些代碼）。

大多數情況下，我們希望開發人員先測試好 CL，確定它們能夠正确運作。但作為評審人員，你仍然要考慮一些邊緣情況，比如查找并發問題，嘗試像使用者一樣思考問題，并找出隻是通過閱讀代碼無法看到的錯誤。

如果願意，你也可以驗證一下 CL。如果一個 CL 會影響使用者，比如做出了 UI 變更，那麼這是驗證 CL 的好時機。如果隻是看代碼，很難了解一些變更将如何影響使用者。對于這樣的更改，如果不友善自己運作，可以讓開發人員提供功能示範。

另一個重要的考慮點是 CL 中是否存在可能導緻死鎖或競态條件的并發問題。隻是簡單地運作代碼很難發現這類問題，通常需要有人（開發人員和評審人員）仔細思考這些問題，確定不會把它們引入到系統中。

複雜性

CL 比實際需要的更複雜嗎？從每一層面檢查 CL，細到每一行代碼，它們是不是太複雜了？函數是否過于複雜？類複雜嗎？“太複雜”通常意味着“閱讀代碼的人難以很快了解它們”，也意味着“開發人員在調用或修改這些代碼時可能會引入 bug”。

過度設計是一種特殊的複雜性，開發人員把代碼寫得比實際需要的更通用，或者增加了系統目前不需要的功能。評審人員要警惕過度設計，鼓勵開發人員隻解決現在需要解決的問題，而不是将來可能需要解決的問題。未來的問題應該在它們出現之後再去解決，因為到了那個時候我們可以看到它們的實際狀況和需求。

測試

要求開發人員進行單元測試、內建測試或端到端測試。一般來說，CL 中應該包含測試，除非這個 CL 隻是為了處理緊急情況。

確定 CL 中的測試是正确、合理和有用的。因為測試本身無法測試自己，而且我們很少會為測試編寫測試，是以必須確定測試是有效的。

如果代碼出了問題，測試會失敗嗎？如果代碼發生改動，它們會誤報嗎？每一個測試都有斷言嗎？是否按照不同的測試方法對測試進行分類？

請記住，測試代碼也是需要維護的。

命名

開發人員是否使用了良好的命名方式？好的命名要能夠充分表達一個項（變量、類名等）是什麼或者用來做什麼，但又不至于讓人難以閱讀。

注釋

開發人員有沒有用自然語言寫出清晰的注釋？他們所寫的注釋都是必需的嗎？通常，注釋應該用于解釋代碼的用處，而不是解釋它們在幹什麼。如果代碼不夠清晰，無法自解釋，那就應該簡化代碼。當然也有一些例外（例如，正規表達式和複雜的算法，如果能夠解釋它們在做什麼，會讓閱讀代碼的人受益匪淺），但大多數注釋都應該指出代碼中不可能包含的資訊，比如這些代碼背後的緣由。

CL 附帶的其他注解也很重要，比如告知一個可以移除的待辦事項，或者一個不要做出代碼變更的建議，等等。

注意，注釋不同于類、子產品或函數文檔。文檔的目的是為了說明代碼的用途、用法和行為。

代碼風格

谷歌為主要程式設計語言和大多數次要程式設計語言提供了代碼風格指南（

http://google.github.io/styleguide/

），是以要確定 CL 遵循了适當的指南。

如果你想對指南中沒有提及的風格做出改進，可以在注釋前面加上“Nit:”，讓開發人員知道這是一個你認為可以改進的地方，但不是強制性的。但請不要隻是基于個人偏好來阻止 CL 的送出。

開發人員不應該将風格變更與其他變更放在一起，這樣很難看出 CL 發生了哪些變化，導緻合并和復原變得更加複雜。如果開發人員想要重新格式化整個檔案，讓他們将重新格式化後的檔案作為單獨的 CL，并将功能變更作為另一個 CL。

文檔

如果 CL 導緻使用者建構、測試、互動或釋出代碼的方式發生了變化，請確定相關的文檔也得到了更新，包括 README、g3doc 頁和其他生成的參考文檔。如果 CL 有移除或棄用代碼，請考慮一下是否也應該删除相關的文檔。如果文檔缺失，要向開發人員索要。

檢視每一行代碼

檢視每一行代碼。有些東西可以看一看，比如資料檔案、生成的代碼或大型資料結構，但不要隻是粗略地掃一下類、函數或代碼塊，并假定它們都能正常運作。顯然，有些代碼需要仔細檢查，至于是哪些代碼完全取決于你，但你至少應該要了解這些代碼都在做些什麼。

如果代碼很複雜或者你難以快速看懂它們，導緻評審速度變慢，你要讓開發人員知道，并在進行進一步評審之前讓他們做一些澄清。如果你看不懂這些代碼，其他開發人員很可能也看不懂。是以，要求開發人員澄清代碼其實也是在幫助未來的開發人員更好地了解代碼。

如果你了解代碼，但又覺得沒有資格做代碼評審，可以確定有資格的 CL 評審人員在代碼評審時考慮到了安全性、并發性、可通路性、國際化等問題。

上下文

代碼評審工具通常隻顯示被修改的代碼，但有時候你需要檢視整個檔案，確定代碼變更是有意義的。例如，你可能隻看到新添加了四行代碼，但如果你看一下整個檔案，會發現這四行代碼位于一個 50 多行的方法中，這個時候需要将這個方法拆分為更小的方法。

你需要基于整個系統來考量 CL。這個 CL 是提升了系統的代碼品質，還是讓整個系統變得更複雜、更不可測？不要接受導緻系統代碼品質退化的 CL。大多數系統都是因為累積了很多小的變更而變複雜的，是以要盡量避免小的變更帶來的複雜性。

好的一面

如果你在 CL 中看到一些不錯的東西，要讓開發人員知道，特别是當他們以一種很好的方式解決了問題。代碼評審通常隻關注錯誤的東西，但其實也應該鼓勵和贊賞好的代碼實踐。有時候，讓開發人員知道他們做對了事情比讓他們知道做錯了事情更有價值。

總結

在進行代碼評審時，你要確定：

• 良好的代碼設計。
• 功能對代碼使用者來說是有用的。
• UI 變更應該是合理的。
• 并行程式設計是安全的。
• 代碼複雜性不要超過應有的程度。
• 不需要實作可能會在未來出現的需求。
• 有适當的單元測試。
• 精心設計的測試用例。
• 使用了清晰的命名方式。
• 清晰而有用的代碼注釋，要解釋“為什麼”，而不是“什麼”。
• 恰如其分的代碼文檔化。
• 代碼要遵循風格指南。

檢查每一行代碼，檢視上下文，確定你正在改進代碼品質，并為表現不錯的開發人員點贊。

檢查 CL

在知道了代碼評審要關注哪些東西之後，如何有效地進行跨檔案代碼評審呢？

• 代碼變更有意義嗎？它們有沒有良好的描述？
• 先看一下代碼變更中最重要的部分，它整體設計得如何？
• 按照适當的順序檢查 CL 的其餘部分。

第一步：從整體檢視代碼變更

先看一下 CL 描述，看看這個 CL 做了些什麼。做出這個變更有意義嗎？如果這個變更是不必要的，請立即做出回複，并解釋為什麼不應該發生這個變更。在你拒絕這樣的變更時，可以向開發人員建議他們應該做些什麼。

例如，你可以說：“看起來你在這方面做得不錯，謝謝！不過，我們正打算移除這個系統，是以現在不想對它做任何修改。或許你可以重構一下另外一個類”？

注意，評審人員在拒絕一個 CL 并提供替代建議時要做得很有禮貌。禮貌是很重要的，因為作為開發人員，我們要彼此尊重，即使可能意見不一緻。

如果有很多 CL 是你不希望出現的，就要考慮重新調整開發團隊或外部貢獻者的開發流程，以便在開發新的 CL 之前進行更多的溝通。提前告訴人們哪些事情不要做，這比等他們做完了這些事情再把它們扔掉或者進行徹底重寫要好得多。

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

找到 CL 的主要檔案。通常一個 CL 會有一個包含了主要邏輯變更的檔案，也就是 CL 的主要部分。先看看這些主要部分，有助于了解整個上下文，加快代碼評審速度。如果 CL 太大，以緻于你無法确定哪些部分是主要的，可以詢問開發人員，或者讓他們把 CL 拆分成多個 CL。

如果 CL 的主要部分存在嚴重的設計問題，要立即回複開發人員，即使你還沒有時間檢查 CL 的其餘部分。這個時候檢查 CL 的其餘部分可能是在浪費時間，因為如果主要部分存在嚴重的設計問題，那麼其他部分就變得無關緊要了。

為什麼要立即回複開發人員？原因有二：

• 開發人員在發出一個 CL 之後會繼續開始後續的開發工作。如果你正在評審的 CL 存在嚴重的設計問題，他們也需要重寫後續的 CL。是以，最好趕在開發人員在有問題的設計上花費不必要的時間之前告訴他們。
• 大的設計變更比小的變更需要更長的時間。為了讓開發人員能夠在截止日期之前送出代碼，同時又能保持代碼庫的品質，要盡早讓他們開始重寫工作。

第三步：按照适當的順序檢查 CL 的其餘部分

在确認整體 CL 沒有嚴重的設計問題之後，試着按照某種邏輯順序來檢查其他檔案，確定不會錯過任何一個需要檢查的檔案。通常，在你檢查完主要檔案之後，按照代碼評審工具顯示它們的順序來浏覽每個檔案就可以了。你也可以在檢查主要代碼之前先檢視測試代碼，這樣可以對代碼變更有一個大緻的概念。

代碼評審的速度

為什麼代碼評審要快速進行？

在谷歌，我們對開發團隊的整體傳遞速度（而不是針對個體開發人員寫代碼的速度）進行了優化。個體開發速度也很重要，但其重要性比不上整個團隊的開發速度。

如果代碼評審的速度很慢，就會發生以下這些事情：

• 團隊的整體開發速度降低了。如果個體開發人員無法快速地對評審做出響應，可能是因為他們有其他事情要做。但是，如果每個 CL 都要等待一次又一次的評審，那麼其他成員的新特性和 bug 修複就會被延遲，可能是幾天、幾周甚至是幾個月。
• 開發人員開始對代碼評審流程提出抗議。如果評審人員要隔幾天才回複一次，但每次都要求對 CL 進行重大修改，開發人員可能會覺得很沮喪。通常，他們會抱怨評審人員太過嚴苛。如果評審人員能夠快速提供回報，抱怨就會消失，即使他們要求做出的修改是一樣的。代碼評審過程的大多數抱怨實際上可以通過加快評審速度來解決。
• 代碼品質受影響。如果評審速度很慢，開發人員的壓力也會随之增加，因為他們不能送出不甚完美的 CL。緩慢的評審流程還會阻礙代碼清理、重構和對現有 CL 做出進一步改進。

代碼評審應該要多快？

如果你不是在集中精力完成手頭的任務，那就應該在第一時間評審代碼。

對代碼評審做出響應最好不要超過一個工作日。

如果遵循這些原則，那麼一個典型的 CL 在一天内（如果需要的話）可以進行多輪評審。

速度和中斷

有一種情況，即如果你正在集中精力完成手頭的任務，比如寫代碼，那就不要打斷自己去做代碼評審。研究表明，開發人員被中斷之後可能需要很長時間才能恢複到之前的狀态。是以，從團隊整體上看，在寫代碼時打斷自己比讓另一個開發人員等待代碼評審要付出更大的代價。

是以，對于這種情況，可以等到你手頭工作可以停了再開始代碼評審。可以是在完成手頭的編碼任務之後，午飯後，會議結束後，休息結束後等。

快速響應

我們所說代碼評審速度指的是響應時間，而不是 CL 完成整個評審過程并送出到代碼庫所需的時間。理想情況下，整個評審過程也應該是很快的，但單次評審請求的響應速度比整個過程的響應速度更重要。

有時候可能需要很長時間才能完成整個評審過程，但在整個過程中評審人員的快速響應可以極大減輕開發人員對“慢”評審的沮喪感。

如果你太忙了，可以先向開發人員發送一個響應，讓他們知道你什麼時候可以開始評審，或者建議讓其他可以更快做出響應的評審人員來評審代碼，或者提供一些初步回報。

最重要的是評審人員要花足夠的時間進行評審，確定代碼符合标準。但不管怎樣，最好響應速度還是要快一些。

跨時區代碼評審

在進行跨時區代碼評審時，試着在開發人員還在辦公室的時候做出響應。如果他們已經回家了，那麼最好可以確定他們在第二天回到辦公室時可以看到代碼評審已經完成。

帶有注解的 LGTM

為了加快代碼評審速度，對于以下兩種情況，評審人員應該給出 LGTM（Look Good to Me，沒有問題）或者通過，即使他們在 CL 中留下了未解決的問題：

• 評審人員确信開發人員将會處理好評審人員給出的建議和意見。

• 其餘的改動是次要的，不一定要求開發人員完成。

  當開發人員和評審人員處于不同的時區時，最好可以使用帶有注解的 LGTM，否則開發人員可能需要等上一整天才能獲得“LGTM，準許”。

大型的 CL

如果有人給你發了一個很大的代碼評審，而你不确定是否有足夠時間完成評審，通常的做法是要求開發人員把 CL 拆分成幾個較小的 CL。這樣做通常是合理的，對評審人員來說是有好處的，即使開發人員需要做點額外的工作。

如果一個 CL 不能拆分成更小的 CL，并且你沒有足夠的時間進行快速評審，至少要對 CL 的總體設計寫一些注解，并發給開發人員。評審人員的目标之一是在不影響代碼品質的情況下快速對開發人員做出響應，或者讓他們能夠快速采取進一步行動。

持續改進代碼評審

如果你遵循了這些指導原則，并且對代碼評審過程嚴格要求，你會發現，随着時間的推移，整個代碼評審過程會變得越來越快。開發人員知道為了保證代碼品質需要做些什麼，并從一開始就向你發送非常棒的 CL，這樣評審所需的時間就會越來越少。評審人員也學會了如何快速做出響應。但不要為了提高評審速度而犧牲代碼評審标準或品質——從長遠來看，這樣做并不會讓任何事情變得更快。

緊急情況

在一些緊急情況下，CL 必須非常快速地通過整個評審過程，在品質方面會有些許的放松。請參看這些“緊急情況”，看看哪些符合緊急情況标準，哪些不符合。

評審注解

概要

• 禮貌。
• 解釋你的理由。
• 給出明确的方向，指出問題，并讓開發人員決定如何在兩者之間做出權衡。
• 鼓勵開發人員簡化代碼，或者添加代碼注釋，而不隻是讓他們解釋代碼的複雜性。

禮貌

一般來說，禮貌和尊重是很重要的。一個是要確定你的評論是針對代碼而不是針對開發人員。你不一定要一直這麼做，但當你想說一些可能會讓開發人員感到激動或有争議的話時，絕對有必要這麼做。例如：

不好的說法：“為什麼你要在這個地方使用線程，這樣做顯然不會獲得任何好處”。

好的說法：“在這裡使用并發模型增加了系統複雜性，但我看不到任何實際的性能好處，是以這段代碼最好使用單線程，而不是多線程”。

解釋理由

從上面的正面示例可以看出，這樣有助于開發人員了解你為什麼要給出這些建議。你并不一定總是要在評審中提供這些資訊，但如果你能夠為你的意圖、所遵循的最佳實踐或你的建議将如何改進代碼品質給出更多的解釋會更好。

給予指導

一般來說，修複 CL 是開發人員的責任，而不是評審人員的責任。你不需要為開發人員提供詳細的解決方案或者為他們寫代碼。

不過，這并不意味着評審人員就不應該幫助開發人員。你最好可以在指出問題和給予指導之間做出權衡。指出問題，并讓開發人員做出決策，這樣有助于開發人員學到東西，并讓代碼評審變得更容易。這樣還可以産出更好的解決方案，因為開發人員比評審人員更了解代碼。

不過，有時候直接給出指令、建議或代碼會更有用。代碼評審的主要目的是獲得盡可能好的 CL。第二個目的是提高開發人員的技能，這樣以後需要的評審就會越來越少。

接受注解

如果你要求開發人員解釋一段你不了解的代碼，他們通常會去重寫代碼，并把代碼寫得更清晰。有時候在代碼中添加注解也是一種恰當的做法，隻要它不隻是用來解釋太過複雜的代碼。

不要隻是把注解寫在代碼評審工具裡，因為這對于将來要閱讀代碼的人來說并沒有多大幫助。隻有少數情況可以接受這種做法，例如，你對評審的東西不太熟悉，而開發人員的解釋卻是很多人所熟知的。

代碼評審回推

有時候，開發人員會回推代碼評審。他們可能不同意你的意見，或者他們抱怨你太嚴格了。

誰是對的？

如果開發人員不同意你的意見，先花點時間想一下他們是不是對的。通常，他們比你更熟悉代碼，是以可能對代碼的某些方面更了解。他們的論點有道理嗎？從代碼品質角度來看，他們的回推是有道理的嗎？如果是，就讓他們知道他們是對的，這個問題就解決了。

然而，開發人員并不總是正确的。在這種情況下，評審人員要進一步解釋為什麼他們的建議是正确的。

如果評審人員認為他們的建議可以改善代碼品質，并認為評審所帶來的代碼品質改進值得開發人員做出額外的工作，那麼他們就應該堅持。改善代碼品質往往是由一系列的小步驟組成的。

有時候你需要花很多時間反複解釋，但要始終保持禮貌，并讓開發人員知道你知道他們在說什麼。

激動的開發人員

有時候，評審人員會認為如果他們堅持要開發人員做出改動，會讓開發人員感到不安。開發人員有時候确實會感到沮喪，但這種感覺通常都很短暫，之後他們會非常感謝你幫助他們提高了代碼品質。如果你在評審過程中表現得很有禮貌，開發人員一點都不會感到不安，這種擔心可能是多餘的。通常，令開發人員感到不安的是你寫注解的方式，而不是你對代碼品質的堅持。

稍後再解決

一種常見的回推原因是開發人員希望盡快完成任務。他們不想經過一輪又一輪的代碼評審，他們說他們會在後續的 CL 中解決遺留問題，你現在讓 CL 通過就可以了。一些開發人員會做得很好，他們在送出 CL 後立即就開發後續的 CL。但經驗表明，開發人員開發原始 CL 的時間越長，他們進行後續修複的可能性就越小。除非開發人員在送出 CL 之後立即進行修複，否則在通過之後通常不會再去做這件事情。這并不是因為開發人員不負責任，而是因為他們有很多工作要做，而修複工作通常會被遺忘。是以，最好讓開發人員馬上把 CL 修複掉。

如果 CL 引入了新的複雜性，在送出之前必須将其清理掉，除非是緊急情況。如果 CL 暴露了一些目前還無法解決的問題，開發人員需要把 bug 記錄下來，并将其配置設定給自己，這樣它就不會被遺漏。他們還可以在代碼中加入 TODO 注釋，指向已經記錄好的 bug。

抱怨評審太嚴格

如果你之前的代碼評審很放松，然後突然變得嚴格起來，可能會引起一些開發人員的抱怨。不過沒關系，加快代碼評審速度通常會讓這些抱怨逐漸消失。

有時候，這些抱怨可能需要幾個月的時間才能消除，但開發人員到最後通常會看到代碼評審的價值，因為他們看到了嚴格的代碼評審有助于産出優秀的代碼。有時候，抗議最大聲的人甚至會成為你最堅定的支援者。

如果你遵循了上述方法，但仍然會在評審過程中遇到無法解決的沖突，請再次參閱代碼評審标準，了解那些有助于解決沖突的指導原則。

原文連結