Google 如何進行 Code Review - 1

一窺 Google 內部程式審核的指導原則 - The Standard of Code Review

ta-ching chen

1 minute read

 文章目錄

前言 

貢獻開源專案 Fission 時，不僅會被他人審核也常需要審核來自世界各地開發者的 PR，從中體會到不少身為一個軟體工程師、一個專案維護者該有的行為與談吐。 近期 Google 公佈其內部關於「如何進行程式審核」的指導原則， 快速瀏覽相關內容後真的是心有戚戚焉，要是能更早知道這些技巧就不會摸索這麼長的時間。

認為這麼棒的文章沒有讓更多人看到實在非常可惜，因此嘗試自己翻譯來分享給中文的開發者。 若有文句不通暢、翻譯錯誤、typo，還煩請多包涵與指教。(部分找不到適當中文翻譯之專業名詞會以原文呈現)

原文 

The Standard of Code Review (程式審核準則) 

Code review 主要目的在於確保 Google 的程式庫 (code base) 整體健康度隨著時間推移能有所改善。 而所有的審核工具與流程皆是因應這個目的而生。為達到前述目的，勢必需要做出一連串的權衡與取捨 (trade-offs)。

首先，開發者必須能夠在任務上取得進展，若不對程式庫進行任何提交的話，那程式庫永遠不會有任何改善。 此外若審核者 (reviewer) 讓任何提交變得難以進入程式庫的話，則開發者會把「做出改善」這件事本身視為沒有益處，並且未來不再做出任何改善 [1]。

另一方面，審核者必須確保每個 CL (changelist) 的品質，且不會讓程式庫健康狀況隨著時間推移而下降。 儘管每個小改動只會造成品質微幅下降，但隨著時間拉長逐漸累積後，最後終將變成壓垮整體品質的那根稻草。 而這通常是非常棘手的事情，尤其是當團隊面臨時間壓力時，會讓他們認為必須抄小徑才能夠達成原訂目標。

再者，審核者對他們正在審核的程式有所有權與責任 (ownership and responsibility)，確保程式庫能夠維持一致、易於維護，以及其他寫於 What to look for in a code review 的準則。

從中我們得到一個作為我們預期在程式審核過程中出現的原則 (標準):

一般而言，只要更動 (CL) 有助於提昇程式庫整體品質的話，審核者就應該批准它

儘管它並非完美

這點也是位於其他程式審核原則之上最高級的原則。

當然在某些狀況下這個原則也是會受到限制。比方說，某個 CL 涵括審核者不希望出現在系統的功能時，此時儘管 CL 的改動設計非常良好，審核者依然可以拒絕批准它。

主要重點在於「並沒有所謂完美的程式，只有更佳的程式」

審核者不該要求開發者在批准程式前仔細清理、擦拭 CL 每個角落 [2]。 相反的，審核者應該在 CL 的完善與其所帶來更動的重要性間做出權衡。與其追求「完美」，身為審核者更應該追尋「持續性的進步(continuous improvement)」。 只要 CL 對整體系統能改善 「維護性 (maintainability)」、「可讀性 (readability)」、「理解性 (understandability)」的話，就不應該因為它不完美而延遲批准數天甚至數週。

審核者也要習慣於針對 CL 可以改善的地方留下審核評論 (review comment)。如果指出的改正不是非常重要的話，可以在前面加上 Nit: 讓開發者知道該改正是可以選擇性被忽略的 [3]。

Note: 這份文件並非嘗試去證明合併 CL 絕對會造成程式庫品質下降的問題。唯一會發生這種事的應該是在緊急情況 [4]。

  • [1] 原文使用 disincentivized，找到最接近的解釋是 「移除做某些事的益處，導致其他人不想再做」。
  • [2] 將程式碼清理到完全符合審核者標準
  • [3] Nit 全稱為 Nitpick，意指吹毛求疵、雞蛋裏挑骨頭。自己本身會使用 nitpick，比如以下範例。
nitpick

利用 nitpick 告知不在風格指南上的小問題

  • [4] 因緊急狀況而合併不完全符合品質要求的 CL

Mentoring (指導) 

程式審核在教導開發者新的東西如程式語言、框架、通用軟體開發原則時扮演非常重要的角色。適時的留下評論來協助他人學會新東西永遠是件好事， 分享知識亦會讓整體程式品質隨著時間推移下得到提昇。

記得一點，如果你的評論是純粹教育性質的話 (意即並非嚴重到必須去符合文件所描述的標準) 可以在評論前加上 Nit: 或是用其他方式告知這並非強制性需要在該 CL 解決的問題 [1]。

  • [1] 除加上 nitpick 外，若評論牽涉改動並非很重要時且不急著解決時，自己會明確告知開發者說 let's resolve this in another PR，避免 PR 被 block 住。

Principles (原則) 

  1. 以技術事實與資料 (數據) 為根本，否決意見與個人偏好

  2. 關於程式風格的問題，風格指南 是絕對的 [1]。任何沒有列在其上的風格要點 (如空格等等) 則關乎每個人的個人喜好。一般來說，風格要和已經存在的程式保持一致。如果原先不存在任何風格的話，則接受 CL 作者的。

  3. 任何有關「軟體設計 (software design)」的層面，從來都不單純是風格或個人喜好的問題。軟體設計必須要建立在基礎原則之上，並且在眾多原則間做出權衡，而不該僅是個人意見。 有時候若有數個不同的可行方案，只要作者能夠展示 (根據數據或完善的工程原則) 這些方案彼此是相等的 (equally valid)，審核者則應該接受作者的選擇。 否則，設計方案的選擇則應該追尋軟體設計的標準原則來進行。

  4. 如果沒有其他規則適用的話，則審核者可以要求作者和現有的程式庫保持一致風格，只要這樣不會讓整體系統程式的品質惡化的話。

  • [1] 此指 Google 風格指南

Resolving Conflicts (解決衝突) 

在任何衝突發生時，第一步永遠是開發者與審核者雙方根據此份及其他在「CL 作者指南」 以及「審核者指南」的文件來嘗試達成共識。

如果達成共識變得尤其困難時，雙方來個面對面會議 (face-to-face meeting) 或視訊會議 (VC) 或許會有所幫助，而不要僅依靠透過評論來試著解決衝突 [1]。 (如果選擇採用 face-to-face meeting & VC，請確保將最後結論記錄在 CL 的評論裡，以便後人查閱。)

如果上述做法仍無法解決衝突的話，最常見的作法是拉高處理問題的層級。從原先的兩人討論變成團隊討論、團隊領導 (TL) 來權衡 (weight in)、 讓程式維護者 (maintainer) 來決定，又或者由工程經理 (Eng Manager) 介入處理。

千萬不要讓改動因為作者跟審核者間無法達成共識而停留在那邊

  • [1] 純文字有時效率緩慢且容易造成誤解對方的情緒，透過對談方能快速理解與同步彼此的想法。 這種狀況對於遠端工作的團隊尤其重要，自己遇到類似情形會直接開視訊會議跟對方討論，確保彼此想法一致 (on the same page)。

下篇: What to look for in a code review

相關文章

文章內容的轉載、重製、發佈，請註明出處: https://tachingchen.com/tw/
comments powered by Disqus