How to do a Code Review - nycu-xuan/FastShop GitHub Wiki
What to look for in a code review
設計
這個改變的引入時機是否恰當？它是否有在該出現的地方？是否有展現應有的功能？
功能
當改變會直接影響使用者時，例如 UI 的改變，單純看程式碼可能感受不出來，此時 reviewer 可以先實際感受改變的效果，或請設計者提供 demo。
複雜度
程式使否超出它該有的複雜度？這包括行與行、函式和類別。「太複雜」代表讀者無法快速理解程式的意圖，這樣的程式碼容易在往後的改變中引入 bug。 有一種情況是「過度工程」(over-engineering)：設計者將函式或類別的功能過度泛化 (generic)。
我們只需要解決現在目標解決的問題，而不是未來說不定會需要的。
測試
對應的測試（單元測試、整合測試，等等）應該包含在同樣的 PR 中。reviewer 應該確認測試的有效性：是否在程式有問題時會 fail？有沒有可能產生 false positive？有沒有將不同函式的測試分門別類？有沒有遺漏的測試邊界？測試不應該測試自己。 同時測試的程式碼也是需要維護的，因此同樣需要檢視其複雜度，盡可能清楚、簡短且有用。
命名
是否使用有意義的命名。好的命名應該足夠表達其作用，但也不該過長而影響閱讀。
註解
註解不應該解釋程式碼在做什麼，而是解釋為什麼需要這些程式碼或某些決策的原因。同時檢查有無過時的註解，譬如已完成的 TODO。
當你覺得需要寫註解時，多半只是程式碼缺乏表現力；當你必須要寫註解時，請盡可能把它寫好。
例外
正規表達式或演算法有時無法避免使用註解。
風格
我們的專案統一使用 formatter，原則上不再需要擔心程式碼風格。
若有強制規範，以規範為主；若無強制規範，以周邊一致性為主；若無周邊需考量，以設計者偏好為主，而不是 reviewer。
當有必要引進風格調整時，不要將其混入包含功能變更的 PR 中， 這會掩蓋重要的變更，並使 commit 糾纏在一起。
文件
如果改變影響建置程序、使用方式或釋出新的 release，相關的文件需一並更新。
每一行
除了 data file、自動產生的程式碼或大型資料結構，應該看過每一行由人寫出的程式碼，或至少，瞭解所有的程式碼做的事情。 如果覺得有不清楚的地方，可以請設計者進行說明。如果發現自己缺少特定領域的知識，如資安，可以請更熟悉的人員幫忙 review。
例外
如果你是多位 reviewer 的其中一位，只負責 review 部分的程式碼，可以不用看過每一行，但應該在 review 後註明自己看過的部分。
上下文
有時看起來只是新增了幾行程式碼，卻讓其所屬的函式超出了合適的複雜度；除了看更改的地方，也要綜觀其周遭。多數系統都是在許多微小的改變之後變得複雜，要避免引入不必要的複雜度。
不要引入會降低系統品質的改變。
好事
在改變中看到了不錯的地方，尤其是對你的 comment 的回應，別吝嗇於稱讚。
總結
在進行 code review 時，應當確認：
- 程式設計良好
- 此功能對使用者來說有用
- UI 的改變合理且美觀
- 程式碼沒有過度複雜
- 設計者不是在解決想像中的問題
- 有適當且良好的測試
- 設計者使用清楚的命名
- 註解清楚流暢，且解釋 why 而不是 what
- 文件有更新
- 程式碼風格有遵循規範
確保有 review 過被指派的每一行、看過上下文，並且有提升程式碼品質，也記得稱讚設計者做的好事。