Skip to content
返回质量保障

审查评论

写具体、友善和可行动的审查评论使作者无困惑修问题。

为什么

代码审查评论是沟通,沟通风格定反馈被收还是被怨。"这错了"技术对但丧志——作者感被攻、变防御、下审查对立。"这可能是 bug,userId 为 null 时会怎样?"具体(指确切问题)、友善(问非控)和可行动(作者知查和修什么)。好审查评论也教——"此处考虑用 Map O(1) 查找而非 array.find"无傲分享知识。审查目标是更好代码非证审查者更聪明。模糊("这坏")、个人("你总这样")或不可行动("修这"无解释)的评论浪费时间损团队文化。友善审查团队更快产更好代码,因反馈被收和行动而非辩论。

何时使用

每审查评论、团队如何沟通反馈的共享标准。

如何做

1. 具体: 指确切行和确切问题
   - 坏: "这错了。"
   - 好: "42 行: userId 为 null 时抛。加 null 检: if (!userId) return null;"
   - 引代码、参行、名问题

2. 友善: 假设作者聪明且尽力
   - 坏: "你测过吗?"
   - 好: "列表为空时怎样?我没见那情况测试。"
   - 问非控
   - 评代码非人

3. 可行动: 告作者做什么
   - 坏: "这低效。"
   - 好: "这是 O(n²) 因嵌套循环。考虑用 Set O(n): const seen = new Set();"
   - 提修复或修复明确方向

4. 解释为什么非仅什么
   - 坏: "此处用 Map。"
   - 好: "此处用 Map O(1) 查找——array.find 是 O(n) 且这在热循环。"
   - 作者学原因下次可应用

5. 区分阻和非阻
   - 阻: "这有 SQL 注入漏洞——合并前必修。"
   - 非阻: "考虑提取为 helper 复用。(nit)"
   - 用标签: [blocking]、[nit]、[question]、[suggestion]

示例

// 坏审查评论(模糊、不友善、不可行动):
// "这错了。"
// "你测过吗?"
// "这低效。"
// "此处用 Map。"
// "修这。"

// 好审查评论(具体、友善、可行动):

// 正确性:
"42 行: userId 为 null 时 user.name 抛 TypeError。
会话在 fetch 完成前过期时怎样?
加 null 检:
  if (!user) return null;
或抛更描述错误:
  if (!user) throw new NotFoundError('用户未找到');"

// 安全:
"18 行: userId 来自 req.query 直接传 SQL 查询。这是 SQL 注入漏洞——
攻击者可传 userId=1; DROP TABLE users。用参数化查询:
  db.query('SELECT * FROM users WHERE id = $1', [userId])
并验证输入:
  const userId = z.string().uuid().parse(req.query.userId);"

// 性能:
"30 行: 这是 O(n²) 因 findUser 在 map 内调且 findUser 对所有用户 .find()。
1000 项时 1,000,000 比较。考虑建 Map 一次:
  const userMap = new Map(users.map(u => [u.id, u]));
  const results = items.map(item => userMap.get(item.userId));
这是 O(n) 大列表快 1000 倍。"

// 可读性 (nit):
"[nit] 50 行: 变量名 'd' 不清。考虑 'daysUntilExpiry'
使读者无需追逻辑即知代表什么。"

// 问题:
"[question] 60 行: 为何此处超时 30s?默认 5s。
此端点需更长超时有原因吗?若有,可加注释解释吗?"

常见问题

  • ⚠留"这错了"评论不解释错什么或如何修——作者须猜、求澄清、审查多花一天。
  • ⚠混个人批评和代码反馈——"你总忘处理 null"使作者防御,有效技术点失于情绪反应。
  • ⚠每小风格偏好标阻——作者不能合并直到变量从 d 重命名 daysUntilExpiry,审查变瓶颈非质量门。
  • ⚠100 行 PR 留 50 评论——作者压倒、处理易的、重要安全评论失于噪音。

最佳实践

  • ✓具体(引行、名问题)、友善(假设作者尽力)和可行动(提修复或明确方向)——这是每审查评论最低。
  • ✓用标签区分阻问题(安全、正确性)和非阻(风格、nit),使作者阻问题修后可合并分开处理 nit。
  • ✓解释为什么非仅什么——"用 Map O(1) 查找"教作者,"用 Map"是令不传下情况。
  • ✓优先评论——先阻问题、后重要建议、再 nit,使作者先处理最重要反馈不迷失噪音。