跟 Google 學如何 Code Review — (4) 審查速度

Evan Tsai
嗨,世界
Published in
7 min readMar 25, 2020

在知道《如何在審查的 CL 中巡航》之後,第四篇則是要探討「審查的速度」,了解身為審查者該以什麼樣的速度進行審查。希望讀者們有更多的想法也能夠跟我們分享。

Photo by Avi Richards on Unsplash

為何要重視審查的速度

從產品角度來看,我們應該優化的重點是「產品開發的速度」。雖然個人速度是重要的,但團隊/產品開發速度更加重要。

審查太慢時容易發生:

  • 團隊產能降低:團隊中溝通的傳遞極為重要,審查者執行其他任務造成審查停擺在沒有溝通的狀態下尚未做出反應,會造成其他成員等待,嚴重的話甚至延遲了產品開發的時程。
  • 開發者的壓力及反感:當審查者回應時間為數天以上,且伴隨著重大修改的提議,容易造成開發人員的挫折及壓力,而認為審查者『過於嚴厲』。加快審查的回應速度可以讓開發者更快知道哪邊要修改(並且提升程式碼健康度),降低時間及情緒上的壓力。
  • 程式碼健康度減低:審查太慢導致的時程壓力會表現在較低品質的 CL 上,比如說程式碼的清潔度、重構和其他的優化等。
[討論]
審查太慢還有一個缺點:開發者自己都忘了自己的程式碼,這會造成多餘的溝通成本。一種本來只需要從 L2 Cache 載入的資訊放太久過期就變成要去硬碟找的概念。

審查應該要多快

在不中斷個人任務的前提下,盡可能在 CL 提交後進行審查,大原則是變更提交後一個工作天內進行回覆。在此原則下,一個變更有可能在一天內進行多輪的審查與修正。

[討論]
審查其實是個雙向的溝通行為,溝通重點之一就是要及時回應,所以快速的回應是很重要的,已讀不回短期造成對方無謂的等待,長期會影響溝通雙方的信任度。

速度與中斷

有研究證實,當開發人員專注於編寫程式時被打斷,需要額外花很長的時間恢復到原本的開發專注狀態/ 心流(可參考此文)。因此如果你正在專注地處理任務時,就不應該強迫中斷任務轉向回應審查需求。

建議設立一個斷點回覆審閱的要求,可能是當下的任務到一個段落時或午餐後等。

[討論]如果在開發一個大功能需要長時間的專注呢?1. 人的專注力通常無法持續一整天,適時的下斷點,轉移注意力可以增加生產力
2. 試著把大功能切分為小功能一個一個慢慢做,這樣也有助於轉換後快速回到原來的工作狀態,過大的功能有太多的內容需要載入,不小心被中斷後會很慘。

快速回應

當我們講到審查速度,其實指的是回應的速度。理想狀態下整個流程應該要是快速的,但比起快速回應,整個 CL 從提交到審查完成的時間反而是次要的。

有時候整個審查從提交到完成的時間可能會很長,快速的回應可以讓大家維持事情正在進行的感受,有效減低負面的情緒如時間壓力、挫折等。

萬一無暇審查時,仍應安排時間並回應給開發者預計何時進行審查,或是請其他成員幫忙,亦或依照審查要點先作些粗略、大方向地審查。(注意我們仍舊遵從前一要項,先把手上工作做完再行回應)

審查通過代表著認可此 CL 已經符合了「審查標準」,因此我們應該花費足夠的時間來確認這件事情。在此同時,也別忘了快速地回應。

跨時區

遇到雙方有時區差異時,盡量在對方的辦公時間回應或是進行溝通,若當天開發者離開了辦公室,盡量在他隔天進辦公室前回覆。

[討論] 為什麼要在對方辦公時間進行回應?因為審查其實是個溝通的過程,是呼應「審查應該要多快」段落所說,會有來回許多輪的討
論,所以利用雙方都上班的時間能比較有效的討論。
遠端工作會影響審查嗎?理論上沒有用到實體物品的審查,除了較差的溝通效率外,不應該會被影響。

批准與註解

在某些情況下,即使審查者在 CL 上留下還未解決的事項,也應予批准:

  • 審查者確信開發者會把那些尚未解決的事項處理好,且不需進一步的確認
  • 其餘的更改很小,且與開發者無關
[討論] 這裡似乎忘了提到一點,改動再小都應該要有個負責人以及後續處理方式,否則容易該處理的事項因為不知道誰要處理就這麼消失。

當然,審查者應該明確告知開發者未解決的事項會依照上述哪個選項進行。

當開發者和審查者位於不同時區時,批准同時又有未解決項目時必須清楚溝通,避免開發者花費額外的時間確認 CL 是否真正的被批准。

龐大的 CL

當 CL 的過於龐大(判斷基礎:是否有可能有時間一次審查完畢),建議與開發者溝通,要求拆成多個較小的 CL。對於開發者來說會需要額外心力,但對整個審核流程的順暢度會有極大的幫助。(較小的 CL 好處多多,後續會用一整個章節來討論)

如果無法分解為較小的 CL,並且您沒有時間完整審查,則至少要對 CL 的總體設計寫一些評論,然後將其發回給開發者進行改進。作為審查者,目標之一是盡量不要成為審查流程中的障礙,讓開發者能夠迅速採取改善行動,同時確保基本的程式碼健康度。

不斷的練習審查

如果您遵循這些準則,並且嚴格執行程式碼審查,會發現整個程式碼審查過程隨著時間會越來越快、越來越熟練。開發者更了解健康程式碼的樣貌樣貌,CL 的品質越來越高,所需的審查時間越來越少。審查者學會快速回應以及避免不必要的流程拖延。但是,不要因為想提昇速度而在審查的標準或品質上做出讓步,因為從長遠來看,這不會提升整個流程的效率,反而可能還會降低,並留下大量的技術債。

緊急情況

在某些緊急情況下, CL 必須非常快速地、規則較為寬鬆的通過整個審查。接下來我們來看看什麼是緊急情況?哪些情況實際上屬於緊急情況,哪些情況不屬於緊急情況。

什麼是緊急情況?

緊急 CL 應該只是一個「很小」的改動,例如:部署失敗時的 hotfix,以避免整個版本 rollback、修正對使用者有重大影響的臭蟲、處理急迫的法律問題、修補重大安全漏洞等。

在緊急情況下,我們關心的是整個審查過程的速度,而不僅僅是回應的速度。只有在這種情況下,審查者應該把審查速度程式碼正確性(這 CL 可以解決緊急情況嗎?)擺在最優先順位。理所當然的緊急情況的審查應優先於所有其他審查。

但是,在解決緊急情況之後,您應該再次回頭查看此 CL 並回歸一般審查時會做的檢查

[討論]
其實只是順序不同而已,出來跑總是要還的。
一般情況: 開發 → 審查 → 部署
緊急情況: 開發 → 最基本的審查 → 部署 → 完整審查

什麼不是緊急情況?

以下情況並非緊急情況:

  • 希望本周而不是下週發布(除非有一些實際的硬性期限 ,例如合作夥伴協議)
  • 開發者開發這功能已經很長時間了,想快點審核通過
  • 跨時區,想抓在對方進辦公室之前完成
  • 想在週末前完成,開發者會很開心
  • 一位經理交辦,但背後沒有任何硬性期限
  • 回復導致測試失敗或建置失敗的 CL

硬性期限 Hard Deadline

硬性期限代表著:如果錯過期限將會帶來災難性的後果。例如:

  • 為了履行合約,而必須在特定日期完成功能
  • 如果您的產品未在特定日期發布,則將影響產品在市場上的定位
  • 有些硬體製造商定期出貨新版硬體,如果錯過限定的提交日期,則會帶來重大的影響

將發布推遲一個星期並不是災難性的,錯過重要的發表日期可能會造成災難性的後果,但通常不會如此。

大多數截止日期是軟性而非硬性的期限。它們表示希望在一定時間之前完成某項功能。它們很重要,但是通常不應影響程式碼健康度。

犧牲審查品質特別容易發生在較長的產品發佈間隔情境下,此情境下錯過當前發佈期限就得再等一段時間。然而這是個不好的處理方式,重複性的犧牲審查的品質會帶來龐大的技術債。如果開發者習慣在發佈前臨時抱佛腳的話,就容易產生品質低落的審查,此時團隊應該修正流程,讓大型變更能提早提交審查,也才有足夠時間進行審查。

[討論]
硬性期限的條件有點模糊,其實大方向就是:如果預期時會有立即且明顯的價值損失,且這價值損失明顯超越審查所能帶來的價值,則可以視為硬性期限。

下一篇《給予回饋的原則以及後續抗力之應對方針》,在我們討論完審查的速度後我們將會延伸到探討關於審查回饋的準則。

--

--