审查评论
写具体、友善和可行动的审查评论使作者无困惑修问题。
为什么
代码审查评论是沟通,沟通风格定反馈被收还是被怨。"这错了"技术对但丧志——作者感被攻、变防御、下审查对立。"这可能是 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,使作者先处理最重要反馈不迷失噪音。