code-review

Sentry 程式碼審查

審查 Sentry 專案的程式碼時，請遵循以下指南。

審查檢查清單

找出問題

在程式碼變更中注意以下問題：

• 執行時期錯誤：潛在的例外狀況、空指標問題、超出邊界存取
• 效能：無限制的 O(n²) 操作、N+1 查詢、不必要的記憶體配置
• 副作用：影響其他元件的非預期行為變更
• 向後相容性：沒有遷移路徑的重大 API 變更
• ORM 查詢：導致非預期查詢效能的複雜 Django ORM
• 安全漏洞：注入、XSS、存取控制漏洞、機密資料暴露

設計評估

• 元件之間的互動是否合理？
• 變更是否符合現有專案架構？
• 是否與目前需求或目標衝突？

測試覆蓋率

每個 PR 都應有適當的測試覆蓋：

• 業務邏輯的功能測試
• 元件互動的整合測試
• 關鍵使用者路徑的端到端測試

確認測試涵蓋實際需求與邊界情況。避免在測試程式碼中出現過多的分支或迴圈。

長期影響

當變更涉及以下項目時，標記為需要資深工程師審查：

• 資料庫綱要修改
• API 合約變更
• 採用新的框架或函式庫
• 效能關鍵的程式碼路徑
• 安全敏感的功能

回饋指南

語氣

• 保持禮貌與同理心
• 提供可執行的建議，而非模糊的批評
• 不確定時以提問方式表達：「你有考慮過……嗎？」

核准

• 當僅剩次要問題時即可核准
• 不要因為個人風格偏好而阻擋 PR
• 記住：目標是降低風險，而非追求完美程式碼

常見需標記的模式

Python/Django

# 不好：N+1 查詢
for user in users:
    print(user.profile.name)  # 每個使用者一次查詢

# 好：預先載入關聯
users = User.objects.prefetch_related('profile')

TypeScript/React

// 不好：useEffect 缺少相依性
useEffect(() => {
  fetchData(userId);
}, []);  // userId 不在相依陣列中

// 好：包含所有相依性
useEffect(() => {
  fetchData(userId);
}, [userId]);

安全性

# 不好：SQL 注入風險
cursor.execute(f"SELECT * FROM users WHERE id = {user_id}")

# 好：參數化查詢
cursor.execute("SELECT * FROM users WHERE id = %s", [user_id])

參考資料

• Sentry 程式碼審查指南