代碼審查指南
Why? 為何需要這個流程?
程式面: 我們會有一個比較健康的程式碼。
團隊面: 每個成員都能彼此熟悉做的東西以及互相學習的機會。
What? 要看什麼?
Understand 看懂
看懂 PR 的實作(怎麼實作,包含所用的套件或語法)
若是 Bugfix,要確認 root cause 為何,且是否有解決
Functionality 功能
Logic 邏輯
Readability 可讀性
命名:是否夠明確,是否符合團隊規範。
註解:不要寫很多註解,因為程式碼本身就是很好的註解。只有在一些比較容易看不懂、或者特別邏輯的地方我們會寫適當的註解,且註解只會描述為何要有這段程式。如果看到一段蠻 “奇特” 寫法的地方卻沒有附上註解,就應該請對方補。
Modularity 模組化:
模組切分的是否恰當,考量單一職責原則,一個元件的職責或目的應該很明確,像是 View 我就是放單純 UI 的東西,Model 就負責處理資料。
Function 的參數是否過多、過長? 如果是過長那很可能是職責太多太雜,我們應該把這個方法拆分。
Reusability 複用性
Error Handling 錯誤處理
Leak
Complexity
Thread Safe: 要避免 race condition
Completeness: 看有沒有 FIXME
, TODO
忘記修
Testing
How? 流程?
提交者
實作前: 先和團隊討論架構、設計和實作方向,確定整體規劃都沒問題後再下去實作
提交 PR:你自己應該是第一個審查者,自己先看後跑自動化檢查 (Status check, testing, static code analysis),自動化檢查通過以及沒有合併衝突後再進行人為審查。
審查者
如果是比較具規模的 PR 都蠻建議直接把程式碼拉下來用 IDE / Editor 看
關於修改,可以分級,
- 常用的是必要和非必要 Required / Suggested,Required 表示一定要修改或者一定要解釋才會過審查的項目,Suggested 則是不用。
- 提供修改建議的時候,可以盡量提供具體可修改的方向、原因、簡單的範例程式或者相關參考資料,縮短對方自己摸索的時間。
Ref