代码评审的协作艺术:提意见和接受意见的正确姿势
代码评审的协作艺术提意见和接受意见的正确姿势一、Code Review 不是找茬是知识传播。但大部分人把它做成了警察抓小偷评审者这里为什么用 for 循环不用 reduce这变量命名不规范啊SQL 查询没走索引吧提交者内心reduce 可读性更差好吧你命名更好你来写这表总共 200 条数据走什么索引代码评审变成了情绪消耗战越审越累。问题出在对 Code Review 的认知错位评审者以为我要找出所有问题越多意见越负责提交者以为我的代码被攻击了我得全部防守回去这是警察抓小偷模式不是协作模式。二、Code Review 的正确心智模型三、实战操作指南评审者的正确姿势1. 区分意见等级必须写前缀# ❌ 差评模糊、带情绪、没有理由 这里写得不好 为什么不用 XX 模式 命名改一下 # ✅ 好评分类 理由 建议 **blocking**: 这个 SQL 查询在 status 列上没有索引 用户列表页会全表扫描数据量大后可能超时。 建议加 idx_status_created (status, created_at) 复合索引。 **suggestion**: processUserData 这个名字不够具体 考虑改成 enrichUserWithOrganizationInfo 更清晰。 这只是可读性优化不阻塞合并。 **nit**: 第 42 行多了一个空行可以删除。2. 用提问代替断言# ❌ 不好 这个 for 循环性能差用 Set 重写 # ✅ 好 这里的 for 循环在用户量大的时候10000可能 O(n²) 我理解是要去重吗用 Set 或 Map 会不会更好一点 你有什么考虑3. 夸比批多 — 三明治反馈法 正面这个模块拆分得很清晰接口设计得很好 建议错误处理那里可以统一用 ErrorBoundary 包装 正面单元测试覆盖率很高用例写得很清楚4. 不要阻塞风格偏好如果只是代码风格forEachvsfor-ofclassvsfunction标注nit或者直接不评价。这些交给 ESLint/Prettier 处理不要变成人工代码风格警察。提交者的正确姿势1. PR 描述模板## 变更概述 用户设置页面的组织信息展示解决跨团队用户名显示不完整的问题 ## 变更内容 - 新增 OrganizationBadge 组件 - 修改 UserProfileCard 显示逻辑 - 新增 getUserOrg API 调用 ## 测试说明 - [ ] 单团队用户显示单个 badge - [ ] 多团队用户显示多个 badge溢出折叠 - [ ] 无团队用户不显示 badge ## 风险点 - 新增的 API 调用是 N1 查询后续需要优化为批量接口 已和 TL 确认先上线后续迭代优化2. 小 PR早提交一个 50 行改动的 PR 和 5000 行改动的 PR评审质量天差地别。超过 400 行变更的 PR 建议拆分。PR 大小经验法则≤ 200 行理想10 分钟内审完200-400 行可接受30 分钟内审完400-800 行需要 1 小时关注度下降800 行必须拆分3. 被提意见时的回应姿态# ❌ 防御姿态 你不懂这个模块的业务逻辑 我们一直这么写的 这改不了影响太大 # ✅ 开放姿态 好建议我改一下 这里用 for 循环是因为要提前 breakreduce 做不到。 还是保留现有写法 你说得对这里确实有 N1 问题我先加个 TODO下个 PR 修复4. 主动邀请评审不要怕被提意见。每条意见都是免费的知识传授。如果某人的评审质量很高下次主动 tasuqinning 你上次对状态管理那块很熟帮我看下这个 Redux slice 合理吗团队 Code Review 规范一份可拷贝的文档# .github/code-review-guide.yml团队公约 code_review: expectations: first_review: 24 小时内给出首轮反馈 pr_size: 建议不超过 400 行变更 reviewer_responsibilities: - 区分 blocking / suggestion / nit 等级 - blocking 意见必须给出具体原因和修改建议 - assume good intent假设对方出于善意写代码 - 不要因为风格偏好阻塞 PR - 每条意见至少配一条正面评价 author_responsibilities: - 写清晰的 PR 描述做了什么、为什么、风险点 - 小 PR不超过 400 行 - 收到 blocking 意见直接改suggestion 可以讨论 - 被拒绝的 suggestion 解释原因不要只说 不行 - 审批通过后立即合并减少冲突 escalation: when: 评审者和提交者无法达成一致 who: Team Lead 或 Tech Lead 拍板 timeout: 24 小时内处理不阻塞进度四、边界分析与常见踩坑陷阱为什么是坑怎么避免评审意见全是 blocking对方感觉在被攻击区分等级标注前缀PR 太大导致评审疲劳5000 行代码没人认真看400 行以下功能拆分盯着风格问题浪费评审带宽ESLint/Prettier 自动化提交者不解释就直接改失去了讨论学习机会先讨论再修改评审拖太久不批提交者阻塞/堆积冲突24h SLA超时 TL 介入评审者太严厉团队成员不敢提 PR新手友好前 3 个 PR 只提 suggestion谁都不审别人的代码知识孤岛没人知道别人在写啥轮值 结对评审权衡评审质量 vs 交付速度过度评审每个变量名都要讨论 → 一周合并不了一个 PR → 交付速度归零。不评审代码质量直线下降 → Bug 率上升 → 一个月后没人能维护 → 长期交付速度归零。正确平衡安全/逻辑/数据问题必须审blocking架构/设计问题必须审可以讨论性能问题基于数据审有 profiling 结果才提风格/命名问题工具化非 blocking特殊场景处理紧急修复hotfix可以放宽评审要求但至少一个人看过 写清楚为什么必须立刻上线。新人 PR放宽风格要求重点审逻辑和安全。前 3 个 PR 多夸少批。跨团队 PR借用对方的领域知识。你审代码质量对方审业务逻辑正确性。五、总结Code Review 的本质是集体所有权——代码不属于写它的人属于整个团队。每个人的代码都要经过另一个人的眼睛这是 QE 过程更是知识传播过程。三句核心心法评审者提意见是为了帮对方写出更好的代码不是为了证明自己更厉害提交者被提意见是别人花了时间帮你不是攻击你团队Code Review 的质量和速度反映的是一个团队的技术文化一个好的 PR 评论区应该是blocking 问题少而精准、suggestion 有来有回的讨论、nit 快速关闭。不是一地鸡毛的情绪对抗。