代碼審核時我們應該審核什么
注意：在考慮下面的原則時，切記要根據《代碼審核標准》進行考慮。
設計
代碼審核中最重要的事情就是考慮一下變更提交的整體設計。變更提交中各個部分的關聯交互是否合理？這些變更是應該在代碼基線中，還是應該提交到支持庫中？這些變更是否能夠與系統內的其他部分很好的整合？現在是不是加入這個功能的合適時機點？
功能性
變更提交是否實現了開發者的真正目標？開發者期望的對於代碼的使用者是否有好的效果？這里的使用者包含終端用戶（會受到變更提交的影響）和開發者（會在將來使用到這些代碼）。
大部分情況下，我們都希望開發者在提交代碼審核前能夠事先做好充分的測試。但是作為審核者你還是要考慮邊界情況，並發問題，嘗試像用戶一樣思考，確保你審核的這些代碼中不會產生任何bug。
當有必要時你需要驗證變更提交的內容 - 當變更對用戶交互產生影響，比如UI變更。有些時候單純讀代碼是很難真正理解變更產生的影響。類似這樣的變更，如果你直接從代碼很難直接運行，可以要求開發者提供功能的演示。
另外一方面在審核代碼，要特別重視類似並發等特別重要的概念，防止存在引發死鎖或者競爭條件的寫法。這類的問題即使運行代碼也是比較難發現的，常常需要開發者和審核者仔細思考確保問題不會發生。（當可能存在競爭條件或者死鎖時，盡量不要采用並發模型。這會導致代碼審核或者理解都變得更加復雜。）
復雜度
變更提交的內容是否必須這么復雜？每一級都要這樣檢查：單行代碼是否過於復雜？方法函數是否過於復雜？類是否過於復雜？“過於復雜”通常意味着”不能被讀代碼的人快速理解“。同樣也意味着“當開發者嘗試修改代碼時更容易引入bug”。
一種常見的復雜化就是過度設計，將代碼過度通用化，或者加入目前系統不需要的特性。審核者需要特別警惕這種過度設計。鼓勵開發者針對當下需要解決的問題進行處理，而不是認為將來會需要解決的問題。將來的問題應該在出現的時候才需要解決，那時候你才能真正看清它實際的特征和需求。
測試
針對變更內容，應該提供單元測試、集成測試或者端到端測試。大體上，應該在變更提交中包含測試代碼，除非變更提交是因為處理緊急問題。
確保代碼中的測試是正確、合理、有效的。測試代碼不是為了測試本身，我們基本不會針對測試寫測試代碼，我們自己來保證測試代碼是有效的。
當代碼異常時測試是否真的會失敗？如果基於測試進行代碼變更，是否能夠產生負的正向反饋？是不是每個測試方法內的斷言都是簡單有用的？測試代碼在不同的測試方法中的分布是否合理？
切記測試代碼也是代碼，也同樣需要維護。不要因為這些測試代碼不在主代碼中就允許它非常復雜。
命名
開發者有沒有設計良好的命名？良好的命名能夠充分體現自己是什么或者做了什么，反之則會造成代碼不具備可讀性。
注釋
開發者有沒有用良好表述的語言寫了清楚的注釋？是否所有的注釋都是必須的？通常有用的注釋是解釋某些代碼存在的原因，而不是某些代碼在做什么。因為如果代碼不能清楚的自描述，你就應該把代碼處理的更簡單一些。可能存在一些例外（例如：正則表達式，復雜算法常常需要注釋來描述具體的工作內容），但是大部分的注釋應該是說明代碼不能包含的內容，比如做這個決定的原因。
在審核變更前先看一下注釋也可能會有幫助。也許有一個待辦項應該移除掉，一個注釋的建議與實際的變更沖突等等。
注意：注釋和類、模塊、方法等的文檔不同，文檔是為了表達代碼的目的，如何使用，使用時會產生何種效果。
樣式
在谷歌，我們提供了所有主要語言的《樣式指南》，甚至包括大部分的低頻語言。要確認變更提交是遵守了樣式指南的。
如果你希望能夠改進某些在樣式指南中沒有提到的樣式點，在你的評論前加上”Nit:“的前綴，這樣開發者就知道這是一個改進點而不是強制的。不應當因為個人的樣式喜好而拒絕變更提交。
開發者不應當在主樣式調整中混合其他提交。這樣會使得變更提交中具體變化了什么內容，使得合並與回滾也更加復雜，還會引發其他問題。例如一個開發者希望重新格式化代碼，那就應當將重新格式化的部分提交一次，其他的變更在這次之后另外提交一次。
文檔
如果一次變更影響到用戶的構建、測試、相互引用、發布代碼等，需要確認有沒有更新關聯的文檔，包括 READMEs（項目自帶文檔），知識庫頁面，以及其他關聯生成的文檔。如果變更刪除或者過期了代碼，確認相關的文檔是否被刪除。如果文檔沒有更新或提交，要求要補齊。
每行代碼
仔細查看你分配到審核的每行代碼。類似數據文件，生成代碼，或者很大的數據結構之類的有時可以大概看一下，但是手寫的類、方法或者代碼段絕對不要大概看一下，假設不會有太多問題。有些代碼就是比其他代碼需要更加仔細的審核，這就是你作為一個審核者需要決定的，但是至少確保你理解了代碼的邏輯。
如果發現看代碼很困難並且導致審核的速度變慢，你應該讓開發者明白問題所在並且等他們澄清整理之后再嘗試審核。在谷歌我們只雇佣像你這樣優秀的軟件工程師。如果你都無法理解代碼，那么其他的開發者也同樣很難理解。當你要求開發者理清代碼時，也是在幫助今后其他開發者能夠理解代碼。
如果你能理解代碼，但是你覺得自己還不夠資格來做某部分代碼的審核，確保有另外一位有資格的審核者能夠負責，例如安全、並發、可訪問性、國際化等復雜問題。
整體
將變更提交放到一個完整的上下文中來查看一般會更有幫助。代碼審核工具一般都是展示變更附近的代碼。有時候你是需要查看整個文件來確認這些變更是合理的。例如，你只看到添加了四行代碼，但是當你查看整個文件會發現這四行是在一個五十行的方法中，而這個方法應該要拆分到更小的方法中。
同樣將變更提交放到系統的上下文中作為一個整理來思考也是很有幫助的。這份變更是否會提高代碼的健康程度還是是的系統更加復雜、更少的可測試之類的問題？決不接受降低系統健康程度的變更提交。大部分系統都是因為不斷添加的小變更使得整體系統的復雜度不斷提高，所以阻止新增提交中的復雜度提升是很重要的。
閃光點
如果在審核中發現了代碼中的閃光點，請務必告訴開發者，特別是他們用很棒的方式實現了你評審中的要求。審核者往往關注於問題，但是他們也應該欣賞和鼓勵一些優秀的實踐，這些行為在有些情況下更有價值，換言之就是要告訴開發者哪些做對了比告訴他們哪些做錯了更重要一些。
總結
當審核代碼時，你應該確保：
- 代碼應該是經過良好設計的。
- 功能都應該是對於用戶有幫助的。（引用代碼的開發者和終端用戶）
- 任何UI修改都是合理並且展示良好。
- 所有的並發操作都要是安全的。
- 代碼不應該過於復雜。
- 開發者專注於眼前的問題，而不要考慮實現將來可能要解決的問題。（不要過度設計）
- 代碼中包含合理的單元測試。
- 測試的設計都是合理的。
- 所有的命名都很清晰合理。
- 注釋清晰有效，大部分都是解釋為什么，而不是做了什么。
- 代碼有合理的文檔（谷歌內部文檔平台是g3doc[1]）。
- 代碼符合樣式指南。
確保你審核了每一行代碼，聯合了整體上下文，確保你通過的代碼時提高了代碼的健壯性，並且在開發者的閃光點上給了足夠的鼓勵和關注。
下一篇：變更提交審核時的建議路徑
參考Quora的這篇問題What is g3doc，谷歌內部的工程文檔平台，並且和版本控制系統結合。 ↩︎