跟 Google 學如何 Code Review — (3) 如何在審查的 CL 中巡航
在知道《Code Review 時要看什麼》之後，我們也做了《Code Review 練習》。這篇則是要針對「Navigating a CL in review」的內容探討，了解身為審查者該怎麼「看」這份 CL。希望讀者們有更多的想法也能夠跟我們分享。
Navigating a CL in review
摘要 Summary
現在你知道要檢查什麼了，那麼檢查多個文件最有效方法是：
- 這些改動合理嗎？有明確的說明嗎？
- 先看改動中最重要的部分。整理來說設計得好嗎？
- 用適當的順序查看剩下的 CL。
步驟一：全面地了解改動 Step One: Take a broad view of the change
看看 CL 說明以及 CL 大體上在做什麼。這個改動是有意義的嗎？如果壓根不應該進行此改動，請立即回覆並說明為什麼不應進行此改動。當你拒絕這樣的改動時，最好還是向開發者建議他們應該做什麼。
例如，你可能會說：「看起來您為此做了一些出色的工作，謝謝！ 但實際上，我們正朝著刪除您在此處修改的 FooWidget 系統的方向發展，因此我們現在不希望對其進行任何新的修改。 相反地，您可以考慮重構我們的新 BarWidget 類別嗎？」
請注意，審查者不只拒絕當前的 CL 並提出其他建議，還有禮貌地這樣做。這類的禮貌很重要，因為即使我們不同意，我們也希望表明我們對於身為開發人員的尊重。
如果類似這樣的 CL 頻繁出現，代表應該考慮重新設計團隊的開發流程或外部貢獻者的發布流程，以便在撰寫 CL 之前進行更多的交流。在人們做了大量工作，但現在不得不扔掉它們或徹底重寫它們之前，最好先告訴他們「不要做這個」。
步驟二：檢查核心的部分 Step Two: Examine the main parts of the CL
找出此 CL「核心」部分的檔案。 通常，一個檔案的邏輯更改數量最多，那它就是這個 CL 的主要部分。首要先看這些主要部分。這有助於為其他較小的部分提供上下文（context），通常也會加快審查的速度。如果 CL 太大，你無法確定哪些是主要部分，請詢問開發者你應該先看什麼，或要求拆分為多個 CL。
檢查核心時如果發現一些重大設計問題，即使你沒有時間立即審查其餘部分，也應立即發送這些評論。事實上，審查其餘部分可能會浪費時間，因為如果設計問題相當嚴重，那麼正在審查的許多程式碼都將消失並且變得不重要。
立即發出這些關於主要設計的評論有非常重要的兩個原因：
- 開發人員通常會發送一個 CL，然後在等待審查時就直接基於該 CL 進行新工作。如果你正在審查的 CL 中存在重大設計問題，那麼他們也會不得不重新設計其之後的 CL。希望你能在他們基於有問題的設計之上進行過多額外工作之前就抓住他們。
- 重大的設計變更比小的變更需要更長的時間。開發人員幾乎都有截止日期；為了在截止日期之前完成任務，並在代碼庫（codebase）中保留高品質的代碼，開發人員需要盡快開始 CL 的所有重大重製。
討論：何時 submit CL/PR？何時 Review？我們團隊現在的做法是：
1. 在定期的 code review session 跟其他人講解自己正在開發的功能的架構，這件事情在我們的團隊裡會在 PR 發出前發生。
2. 功能剛 push 一兩個 commit 的時候就開 PR。然後 PR 掛上 WIP(work in process)。讓大家除了在專案管理工具（我們是用 Notion）以外，也能知道現在有哪些功能正在開發中。
3. PR Approve 的時機：
3.1 功能完備，主要架構確立的時候。通常在前面幾個 commit 就會確立架構，後續 commit 就是將功能完善。
3.2 整個功能連骨帶皮都做完的時候。通常是在確認這個功能要 merge 之前，針對細項做微調。目前我們發現，人多了之後在 code review session 講述功能架構會是比較耗時的方式。
儘管如此，我們還是希望更多工程師加入我們的團隊。
步驟三：按適當的順序查看 CL 剩下的部分 Step Three: Look through the rest of the CL in an appropriate sequence
確認 CL 整體沒有大的設計問題後，請試著找出邏輯順序來查看檔案，同時還要確保沒有錯過審查任何檔案的功能。通常，在瀏覽完主要檔案後，依照審查工具向你顯示的順序瀏覽每個檔案是最簡單的。有時在閱讀核心程式碼之前先閱讀測試也是有幫助的，因為這樣你就可以了解改動的含義。
在看完了前三章節以及一篇練習之後，各位對於 Code Review 有什麼想法都歡迎留言一起討論。
