[TOC]

## Guides for Design Review & Code Review

### Pros

- Early identification of design issues when making changes is still cheap.
- Achieving consensus around a design in the organization.
- Ensuring consideration of cross-cutting concerns.
- Scaling knowledge of senior engineers into the organization.
- Form the basis of an organizational memory around design decisions.
- Acts as a summary artifact in the technical portfolio of the software designer(s).

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

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

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

### Review 有哪些形式？

:::info
參考 [來一場兼顧程式碼品質及開發效率的 Code Review - 比較表：各種 Code Review 形式的特性與應用](https://www.thingsaboutweb.dev/zh-TW/posts/code-review#%E6%AF%94%E8%BC%83%E8%A1%A8%E5%90%84%E7%A8%AE-code-review-%E5%BD%A2%E5%BC%8F%E7%9A%84%E7%89%B9%E6%80%A7%E8%88%87%E6%87%89%E7%94%A8) 了解更多不同的 code review 形式，及適合的情境。
:::

#### 同步形式
同步的 Review 是即時進行的，需要兩個以上的人同時參與：

- **Pair Programming**：兩人一組即時協作，一人負責編寫程式碼，另一人即時審查和提供建議。
- **Live Review**：開發者與 Reviewer 即時同步進行程式碼審查與討論的一種方式，透過即時互動快速解決問題並確認程式碼品質。
- **Code Review Meeting**：團隊成員集體參與的審查會議，針對關鍵程式碼變更進行深入討論，以獲得多方視角並提升產出品質。
- **Code Walkthrough**：偏向知識交流性質的互動，由開發者逐步解說程式碼的邏輯與功能，團隊成員主要是以觀察與提問的角色參與，目的是提高對程式碼的理解和學習。

#### 非同步形式
非同步的 Code Review 則不需要參與者即時作業，所有的審查和回覆都透過數位載體進行，例如：

- **PR Review**：開發者提交 Pull Request，Reviewer 可依據自己的時間查看代碼變更、留下評論並進行交流。

:::success
:heavy_check_mark: 團隊還不大的時候，我們優先選擇 Live Review。是否要執行 Code Review Meeting 及 Code Walkthrough，取決於 author 的判斷。
:::

:::danger
:question: 團隊不大的時候，何時使用 PR review？
1. changes 很單純、很小的時候
2. 團隊彼此對 code base 足夠熟悉的時候
:::

### Design review 的關注點

先參考 [Design Docs at Google](https://www.industrialempathy.com/posts/design-docs-at-google/)，了解何謂 design doc 及它可能包含的內容，並將 design doc 作為進行 design review 活動時的主要素材。

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

:::info
:information_source: 一個 checklist，參考自 《微服務實戰 (Microservices in Action)》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 兩者立場應關注的面向。

#### 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

- 應在 PR **title** 對你的改動有簡短的總結，並提供足夠清楚的訊息於 PR description 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 更多的背景，以讓雙方在技術事實上達成共識**

### References
- [Design Docs at Google](https://www.industrialempathy.com/posts/design-docs-at-google/)
- [Code Review Developer Guide at Google](https://google.github.io/eng-practices/review/)
- [來一場兼顧程式碼品質及開發效率的 Code Review - 比較表：各種 Code Review 形式的特性與應用](https://www.thingsaboutweb.dev/zh-TW/posts/code-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)