[TOC]

## Guides for Design Review & Code Review

> 本文主要摘要 [Google’s Code Review Guidelines](https://google.github.io/eng-practices/review/) 中的重點。
>
> 其他延伸閱讀：
> - [如何進行 Code Review?](https://enginebai.medium.com/code-review-guidelines-b76a859c377c)
> - [How to Review the Code Like a Pro
](https://medium.com/@yar.dobroskok/how-to-review-the-code-like-a-pro-6b656101eb89)
> - [從 Code Review 的小事看到大事](https://william-yeh.net/post/2023/09/on-code-review/)
> - [Best practices for writing code comments from StackOver flow](https://blog.taiwolskit.com/best-practices-for-writing-code-comments)

### Pros
- 知識共享
- :point_right: 滿足小夥伴們的成長需求
- 凝聚共識、降低理解成本
- :point_right: 提高系統的可讀性、可維護性
- 揭露各種 hidden knowledge、掌握風險
- :point_right: 減少意料之外的 bugs/issues 產生

### Cons
- 在設計開發階段花費較多時間

:::danger
:fire: 但通常在軟體開發領域，事前先花這些時間 code review、提早找出問題的時間成本，遠比軟體上線後發生再修正、再維護所花費還要低
:::

### 誰是你最佳的 Reviewer(s)?
- 可能是程式碼的 owners
- 但也可以針對特定程式碼段落，尋找最適合的人來跟你討論

### Review 的形式不拘
- in-person (sync mode, pair programming, pre-committed review)
- online (async mode, inline comments, IM notifications)

### Design review 的關注點

我的話，會關注以下面向：
- [x] 我了解每個 public API 的職責嗎？
- [x] 我了解每個依賴的外部系統的職責嗎？
- [x] 是否使用適合的、易理解、好維護的架構/技術棧？
- [x] 我了解目前的架構「妥協」的項目嗎？這些被妥協的項目何時會變得又重要又緊急？
- [x] 我了解目前的規劃接受了哪些「風險」嗎？

:::info
:information_source: A design review RFC example (ref. 《微服務實踐》P.309)

|Aspects|Purposes|
|-|-|
|問題與背景|這個功能想要解決的問題？ 為什麼我們要這麼做？|
|解決方案|打算如何解決此問題|
|依賴項目與整合|如何與現有的或預計要開發的服務、功能及組件進行互動|
|API 接口|服務會暴露哪些操作|
|擴展性與效能|該功能如何進行擴展？維運成本試算？|
|系統可靠度|希望可靠度到達什麼程度？|
|冗余性(redundancy)|備份、復原、部署與回滾|
|監控儀表板|如何了解服務的行為？|
|故障情境|如何因應或消除潛在的故障影響|
|安全性|威脅樣態、資料保護等|
|部署|如何使該功能上線|
|風險與開放問題|目前已識別了哪些風險？有哪些是開發者不知道的風險？|
:::

### Code review 的關注點
以下內容摘要自 [Code Review Developer Guide at Google](https://github.com/google/eng-practices/blob/master/review/index.md)，並分成 reviewer 以及 author 兩者立場應關注的面向。

內容都不長，再自己讀一下，偷學 Googler 怎麼做事。

#### How To Do A Code Review? *(as a reviewer)*

> *A practical checklist from [Code Review Developer Guide: What Do Code Reviewers Look For? - at Google](https://github.com/google/eng-practices/blob/master/review/index.md#what-do-code-reviewers-look-for-lookfor)*

|Aspects|Purposes|
|-|-
|Design| 程式碼是否精心設計且適配你的系統？ <br> *Is the code well-designed and appropriate for your system?*
|Functionality| 程式碼是否符合作者的意圖？是否顧全 user 的利益？ <br> *Does the code behave as the author likely intended? Is the way the code behaves good for its users?*
|Complexity| 程式碼可以更簡潔嗎？其他人是否可以輕易理解並應用？ <br> *Could the code be made simpler? Would another developer be able to easily understand and use this code when they come across it in the future?*
|Tests| 程式碼是否有正確且設計良好的自動化測試？ <br> *Does the code have correct and well-designed automated tests?*
|Naming| 變數、類別、方法命名是否清晰？ <br> *Did the developer choose clear names for variables, classes, methods, etc.?*
|Comments| 註解是否清晰且有用？ <br> *Are the comments clear and useful?*
|Style| 程式碼是否遵循我們的慣例？ <br> *Does the code follow our style guides?*
|Documentation| 相關文件是否有更新？ <br> *Did the developer also update relevant documentation?*

#### The CL Author's Guide *(as an author)*
> CL: change list. interchangeable terms: code changes, MR (merge request), and PR (pull request)

***Writing good CL descriptions***

> see [original page](https://github.com/google/eng-practices/blob/master/review/developer/cl-descriptions.md) for some good and bad examples

- 應在 title 對你的改動有簡短的總結，並提供足夠清楚的訊息於 body
- 而小改動 (i.e. only 1 line)，需要闡述清楚的是 context

***Small CLs***

每次改動範圍小，好處多多：
- reviewer 被打斷的時間更短，review 更快、更詳盡、周全
- 容易設計的更好，再寫出 bug 的機率較低
- 若被 reject，工作量也不會太大
- 容易 merge、容易 rollback (revert)

***How to handle reviewer comments***

:point_right: ***別往心裡去 (Don't Take it Personally)***

- 記得，大家都是為了如何讓程式碼品質上升、維護好 code base 及產品的品質而努力。絕非針對你個人的能力或人格作出攻擊或評價
- 當然， reviewer 也有責任提出有建設性的建議，而不要作出不禮貌或違反職場禮儀的評論
- 若你覺得 reviwer 有任何冒犯之處，請嘗試像他表達你的感受。如情況未改善，請找更高層的 manager 反應此情事

:point_right: ***修正程式碼 (Fix the Code)***

- 如果 reviewer 提出了一段 code 他看不懂，表示很有可能其他 developers 也會看不懂
- 如果你同意 reviewer 的意見，此時你應該是修改程式碼撰寫風格使其更簡潔清晰，或在程式碼中加入適當的註解以解釋為何存在。而不是寫在 code review tool (e.g. Gitlab)，因為其他 developers 在未來看到的是 code 而不是當年的 MR。
- 當然，如果你不同意，或 reviewer 的意見真的無關緊要，直接回應在 code review tool 上就可以了

:point_right: ***自我覺察 (Think for Yourself)***

- 你花了很多力氣寫 CL description，你當然希望 reviewer 少廢話、給我過。但請收到意見回饋時退一步思考該 reviewer 的建議有沒有價值。此時可放在心上的第一個疑問是：「reviewer 的意見到底正不正確？」
- 如果你無法判斷 reviewer 的建議是否正確，**請向 reviewer 要求再進一步的澄清、說明**
- 如果你已經考慮過，並且仍然認為自己的判斷較合適，**請隨時準備好解釋為什麼你的做事方法對 code base、users、products 與/或 company 更好**
- reviewer 的確很可能缺少足夠的上下文來做出最正確的評論，你也確實知道一些 reviewer 不知道的細節。**試著給 reviewer 更多的背景，以讓雙方在技術事實上達成共識**