# 來一場兼顧程式碼品質以及開發效率的 Code Review - Fin

{%hackmd LeyMdnM3Q4ipfr57bkqpyA %}

## [📚 議程介紹](https://webconf.tw/agenda/day1-3-f)

## 簡報
[投影片](https://slides.com/finchen/code)

[TOC]
## ▼▼▼ 開始筆記 ▼▼▼

code review 的期望
* 促進團隊合作與知識分享
* 提升程式碼品質 可維護性
* 確認有照規格完成減少bug
* 佈署有信心

透過知識的視角交流

現狀與挑戰
1. 非常耗時，平均6hr/wk
2. 平均耗時數日，大範圍變更需要更久
3. 找出問題只有14%

code review 很好但很耗費時間和精力

兼顧程式碼品質以及開發效率的 code review 要怎麼做？

三個角度來看 code review

1. 技術執行面
code review 本身的進行
code review 的形式

2. 知識交流面 (Reviewer 跟開發者交流、相關知識的傳遞)

3. 整體開發面
從整體開發流程的其他地方來幫助

Code Review 的形式


- 同步
- pair programming
- 有經驗帶沒有經驗
- 兩個同經驗的
- live review
- 找人過來一起看
- code review meeting
- 開會議一起看
- ~~公開處刑~~ 公審
- code walkthrough
- 找熟悉的人一步一步手把手看，比較快的進入狀況
- 非同步
- PR Review

### 各種code review優劣比較

# 審查方式比較

| 形式 | 特點 | 適用場景 | 優點 | 缺點 |
| ------------------- |:------------------------------------------------------------ | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------ | ---------------------------------------------------------------- |
| Pair Programming | 兩名開發者同步合作，一人寫扣一人審查和提供建議 | - 複雜功能開發<br>- 高風險代碼的實作<br>- 新人培訓 | - 即時反饋，快速解決問題<br>- 減少缺陷<br>- 促進知識共享 | - 需要雙人同步時間<br>- 對時間和人力要求高 |
| Live Review | 同步進行的審查，Reviewer 和開發者即時交流並討論細節 | - 緊急變更需要快速確認<br>- 複雜、需要深入討論<br>- 新人指導 | - 即時互動，減少溝通成本<br>- 適合高複雜度的問題<br>- 提高團隊信任 | - 可能導致討論發散<br>- 占用多人同步時間 |
| Code Review Meeting | 團隊集體參與，針對關鍵程式碼進行深入討論 | - 重大架構或設計變更<br>- 高風險功能<br>- 團隊知識共享 | - 多方視角，檢查更全面<br>- 促進團隊學習<br>- 提高設計透明度 | - 時間成本高<br>- 不適合日常小型變更 |
| PR Review | 非同步的審查，通過工具（如 GitHub/GitLab）查看變更並留下評論 | - 日常開發的變更審查<br>- 遠端或跨時區團隊 | - 時間靈活<br>- 有審查記錄可供日後查閱<br>- 可結合自動化/AI 工具提升效率 | - 缺乏即時互動，容易造成溝通不暢<br>- 複雜變更需要花更多時間理解 |
| Code Walkthrough | 同步進行，開發者逐步解說邏輯和設計，Reviewer 提出問題和建議 | - 新人指導<br>- 複雜程式碼的學習<br>- 熟悉系統未知區塊<br>- 團隊知識共享 | - 有助於新人成長<br>- 幫助團隊理解系統的複雜邏輯<br>- 促進開發者反思自己的程式 | - 偏重於知識傳遞，審查效率不如其他形式 |

### 象限圖

緊急又簡單 推就對了
~~使用者就是QA~~

Principle of least astonishment-最小驚訝原則
>簡單來說，就是降低使用者的學習成本。當你看到某些功能和設計時，不會感到驚訝，因為你已經熟悉它們的使用方式。寫程式也是同樣的道理，目的是實現功能、解決問題，而不是為了炫技。如果可以用簡單的方法達成，就不要把事情弄得複雜，以免適得其反。

https://arc.net/l/quote/ixwcuhtc

## code review 提升執行面效率
* 選擇正確的 Code Review 形式
* 自動化：靜態程式碼分析 + 測試 + CI 確保程式碼基本品質
* AI 化：幫助找出一些基本錯誤
* 主要精力放在實作跟邊界的審查

coderabbitai 開源的免費code review
> https://www.coderabbit.ai
> 填寫表單有 3 個月免費
> https://www.coderabbit.ai/startup-program

## AI code review
1. 定義清楚規格書對於AI產出有正面影響
2. 合理變更範圍，對coding style, api參數， 錯誤處理方式等建議都很實用
3. 不要期望AI可以有假設性的建議
4. 檔案數量太多AI會有品質下降的問題

## 知識交流面

### 正確的理解後才能正確的給予回饋

- 先閱讀 PR 的描述和提交記錄，確認自己是否理解這次變更的目標
- 參考相關測試與文件、規格說明
- 若有疑問，主動提問，要求補充背景資訊
- 用最有效率的方式來獲取資訊
- 討論過程中的重要資訊記得整理與記錄

## 交流要點

### 要點1：
- 同步>非同步
- 面對面> 線上
- 正面語氣 >負面
- 在code介紹可以連結到文件資料

### 要點 2：
- 預想大家都想把事情做好 => 比較容易使用正面語氣

### 要點 3:
- 明確、可行動的
- 針對不同的能力背景，給予詳細或簡潔的回覆

#### 範例 1 (bad)
建議在 `calculateCAGR` 函式中加入對 `starting_value` 和 `ending_value` 是否為零的檢查。目前函式在輸入為零時會因除以零而拋出 `ZeroDivisionError`。

#### 範例 2 (good)
「建議考慮一些邊界情況。」

## 開發流程面

在功能規劃階段，把規格定義好並記錄清楚
（組員同步規格，減少溝通時間）

PR越大 code review效益越低

PR 與審查效用成反向

pr超過五百行的時候，審查時間就降低了
~~(因為懶的看了)~~

>lgtm

## PR減肥

1. 功能規劃階段把規格、相關文件理清
2. 把需求拆成 **半天** 可以完成的小任務
3. 任務相關性拉出來
4. 依據相關性實作，一個任務一個 PR
5. 關鍵字： Stacked PR (每個 PR 都基於前一個 PR 的程式碼碼基礎進行構建，形成一個「堆疊」結構。這種方法可以讓代碼審核變得更清晰，並將大型變更分解為更小的、易於管理的部分)
6. 目標： LOC < 500

> LOC : Lines of Code的縮寫

> Stacked PR:（疊加的 Pull Requests）是一種將程式碼變更分成多個有條理、循序漸進的 PR（Pull Request）的方式，這些 PR 是相互依賴的，組成一個「堆疊」結構。

## Github copilot action

GitHub 提供 Copilot Actions 功能，協助用戶快速編輯和生成程式碼相關的文檔或內容。

## 不一定需要 Code Review的情況

- 對改動的信心足夠
- 測試足夠 或 QA 足夠
- 實驗性質的內容

Pr 的挑戰

---

## 聊天室

我都跟小鴨鴨code walkthrough
小鴨鴨說他不懂
那個表格是複製貼上還是ＡＩ產生啊ＸＤＤ

表格超威 應該是AI 不是手打吧xD
好強ＸＤ
圖片貼不上去了
重新整理再貼看看
可以試試看把圖片壓縮

還是聽不懂邊界是什麼？？有人知道嗎，是類似API的地方嗎
邊界測試檢查（？
在交流要點 3 的範例

邊界 >> 改動範圍有影響到外界的部分，可能是 API （影響到使用 API 的）也有可能是 UI（接觸到使用者）、甚至 DB 欄位、或只是內部某個模組（接觸到使用這個模組）。我們會希望邊界是盡可能的小，這樣改動所造成的影響也會比較小

code review 會遇到的 三大問題
1. 一次的code 不要太多，
2. 面對不熟的程式碼。(比如 後端 review 前端code)
3. libaray 的使用與改寫，比如 datatime 要用 monent.js 還是原生
4. 專案在趕

後端review前端會吐血吧ＸＤＤ
程式語言不同怎麼 review（？
都是 .ts
哦哦，NodeJS 嘛ＸＤ
yes
後端 用 nestjs ，前端用 nextjs
domain還是有差ＸＤ

monent.js不是不維護了
時代的眼淚

四天王有五位

linus就是派
~~被他罵會不會反而覺得被關注了
~~
抖Ｍ（？

個人淺見，邊界大概像是這隻api 需要得到的資訊及回傳的資訊是否太多或太少 ex. user api 拿到商品資訊（？

看到jetbrain有出現毛毛蟲就代表該注意了

Rudy 之前的 Talk 有說 一個 commit 希望 LOC < 120，彙整這邊一個 PR 的 LOC < 500