跟 Google 學如何 Code Review — (1) 基本原則
Code Review (程式碼審查,以下簡稱審查,負責審查的人稱作審查者)是軟體團隊開發時的一個重要環節。我們團隊希望能在這件事上做得更為精進,因此選擇研讀 Google 前陣子公開的程式碼審查指南[註1],此次的系列文會紀錄我們研讀的內容以及相關討論,期望在自我成長的同時也能激起讀者們更多想法。
The Standard of Code Review
程式碼審查的最主要目的是:提升程式碼的品質。因此,所有相關工具 ex: Coverage / Linter 應該以此目的為方向來使用。
為了達到此目的,有些需要取捨的地方:
- 提升品質是個過程,如果審查者標準過高,造成 CL (Change List 程式碼變更) 停滯,則不會有任何的品質提升,長久下來還會降低開發者改善程式碼的意願。
- 但審查者的工作就是要維護程式碼品質,而程式碼品質都是從一些小地方變髒的,就如同房間是慢慢的累積灰塵逐漸變髒一樣。我們當然不想等到髒到不行再來打掃,而是每天每次 CL 都打掃一下。這就很考驗審查者功力,要找出細微的不理想的部分(尤其是有時程壓力時),並且請開發者修正。當然,程式碼的一致性、可維護性是基本。
審核標準:正確完成功能規格,且程式碼品質沒有降低。
[討論&想法]
這裡所謂的『增進程式碼品質』是很模糊的概念,可以分兩塊來看:增進以及程式碼品質。1. 增進:對於一個 CL, 審查者可以給出各種意見,但這些意見必須要是可行的否則只會造成 CL 的停滯。所謂的『可行』應該有一些特性:
- 具體 - 需要改的段落、建議修改方向及參考資料
- 考慮時程 - 除非影響到整個功能完成度,否則不應該提出大型的架構修改要求。會影響到時程的修改要求可以考慮排入未來工作規劃。
- 換個角度說,開發者看了你的回饋之後是否能在有限的時間增加程式碼的品質?
2. 程式碼品質:包含可自動化跟不可自動化兩種
- 不可自動化 → 可維護性:未來需要新增修改的容易程度
- 不可自動化 → 可讀性: 函式邏輯是否直覺,若本身就不直覺是否有相關參考資料?
- 可自動化 → 可測試性: 透過 Code Coverage 等量測
- 可自動化 → 格式: 透過 coding guideline 之類工具檢測
沒有完美的程式碼,只有更好的程式碼。審查者不需要確保程式碼完美無缺,而是確保程式碼/功能有在持續的進步與改善。因此,在給予回饋時,如果僅僅是個人意見或是非必要的更動,則標明『非必要』,讓開發者自行決定如何處理。
註1: 只有在緊急狀況時,才可以提交明顯會降低程式碼品質的 CL,因為此時最高目標是快速的恢復系統正常運作。在其他狀況下如果沒有危害到系統運行,則程式碼的品質是優先考量(包含系統恢復正常後的品質修補)。
互相學習
審查的過程可以是很好的互相學習過程,無論是語言框架特性、或是設計概念,身為審查者的時候如果有不同的想法且值得討論或傳授的,就盡情的回饋吧。透過分享增長成員之間的知識,也可以為未來的程式碼品質打下基礎。但一樣要記得區必要/非必要的意見回饋。
[討論]程式碼審查的產出除了更好的程式碼品質之外,也是知識轉移/形成共識的過程。審查過程中如果能討論出新的共識,則應該記錄在知識庫中,變成組織內的共享知識。這樣有助於大家共同提升知識水準,且未來如果對此共識有其他討論時,也可以有更多參考的資訊以及依據。每個人的喜好都不一樣,因此就算有了共享知識,也不一定馬上會有一致的做法。但至少透過互相學習可以理解各種不同的想法,當成員都能保有這樣的心態時,大家的知識就會不斷的擴展,也有助於我們產出更好的程式碼。
基本原則
- 事實及數據為主,個人偏好次之。
- 務必遵守 style guide 。這是維持程式碼可讀性的最基本要求,如果 style guide 沒有提,則遵守現行 codebase 的(但建議如果此規則 codebase 內已經統一,可以放到 style guide 內)。如果都沒有則尊重開發者選擇(但可以就需求討論)。
- 程式流程或架構設計因為較為繁雜,需要有較深入的討論,大體上如果開發者可以證明設計是可以符合功能需求及兼顧可維護性等,則以開發者選擇為準。
- 不使現行程式碼品質降低的前提下,如無其他考量,基本要求是維持程式碼的一致性。
意見衝突
如果審查者與開發者意見不同而不能解決?
大原則是:試著找出雙方都可以接受的解決方式,詳細可以參考開發者指南以及 Reviewer 指南。
如果探討的議題範疇較大,建議以面對面/視訊這類溝通方式較有效率,但記得討論的決議要紀錄回到 CL 討論串上。如果仍無法解決則尋求外部協助,包含較高層級的負責人、或是領域內較資深者等。
處理重點:討論後務必做出決議,不要讓 CL 掛在那邊沒人處理。
[討論1]意見衝突其實很是訓練團隊信任度以及的思想開放程度的好時機。互相信任及思考較為開放的團隊在衝突時會偏向討論哪個方式可以更好地完成目標。反之較為堅持主見的人則容易遇到衝突無法解決的狀況,因此程式碼審查也可以看作是培養團隊溝通討論的風氣的一個方式。[討論2]所有討論最重要的就是要有決議,沒有決議的話等於維持現狀,等同沒有改變。因此在討論後一定要確定此次討論的結論,並且確定大家的認知是相同的。
接下來,就讓我們看看審查時該注意的重點
附註與參考
- 此指南已經有中文翻譯。