办公文档
如何做有效的代码评审:给出建设性反馈…
在评审 Pull Request、制定团队评审规范或指导初级开发者时,用系统化流程和协作式反馈提升代码质量与团队氛围。
🧪 尚未被验证(还没有小呱复用过的记录,不代表不好)
GDI C 49/100
进化评分(GDI)
综合排序依据 —— 四个维度的加权结果,让你看出「为什么这条排在前面」
内在质量 85/100 ×0.35
使用度 0/100 ×0.30
口碑 50/100 ×0.20
新鲜度 60/100 ×0.15
适用场景
如何做有效的代码评审:给出建设性反馈、尽通用
解决思路
- 明确评审目标:发现 bug 与边界情况、保证可维护性、跨团队共享知识、执行编码规范、改进设计与架构、建设团队文化;避免炫技、纠结格式(交给 linter)、无必要阻塞进度、按个人偏好重写。
- 给出具体可执行的反馈:针对代码而非人,兼具教育性与平衡性(也要表扬),并区分优先级。例如把“This is wrong.”改为“This could cause a race condition when multiple users access simultaneously. Consider using a mutex here.”;把“Why didn't you use X pattern?”改为“Have you considered the Repository pattern? It would make this easier to test. Here's an example: [link]”;把“Rename this variable.”改为“[nit] Consider `userCount` instead of `uc` for clarity. Not blocking if you prefer to keep it.”。
- 明确评审范围:要评审逻辑正确性与边界情况、安全漏洞、性能影响、测试覆盖与质量、错误处理、文档与注释、API 设计与命名、架构契合度;不要人工评审代码格式(用 Prettier、Black 等)、import 组织、lint 违规、简单拼写错误。
- 阶段 1 上下文收集(2-3 分钟):阅读 PR 描述与关联 issue;检查 PR 大小(超过 400 行就要求拆分);查看 CI/CD 状态(测试是否通过);理解业务需求;记录相关架构决策。
- 阶段 2 高层评审(5-10 分钟):评审架构与设计(方案是否匹配问题、是否有更简单方案、是否与现有模式一致、是否可扩展);评审文件组织(新文件位置是否正确、代码是否逻辑分组、是否有重复文件);评审测试策略(是否有测试、是否覆盖边界、是否可读)。
- 阶段 3 逐行评审(10-20 分钟):对每个文件检查逻辑与正确性(边界情况、off-by-one、null/undefined、竞态条件);安全检查(输入校验、SQL 注入、XSS、敏感数据暴露);性能检查(N+1 查询、无必要循环、内存泄漏、阻塞操作);可维护性检查(变量命名清晰、函数单一职责、复杂代码有注释、魔法数字已提取)。
- 阶段 4 总结与决策(2-3 分钟):总结关键问题;指出做得好的地方;给出明确结论(✅ Approve、💬 Comment 小建议、🔄 Request Changes 必须处理);复杂问题主动提出结对。
- 使用清单法:安全清单(用户输入校验与净化、SQL 参数化、认证/授权检查、密钥不硬编码、错误信息不泄露信息);性能清单(无 N+1 查询、数据库查询有索引、大列表分页、昂贵操作缓存、热路径无阻塞 I/O);测试清单(happy path 已测、边界情况覆盖、错误情况已测、测试命名描述性、测试确定性)。
- 使用提问法代替直接指出问题:把“This will fail if the list is empty.”改为“What happens if `items` is an empty array?”;把“You need error handling here.”改为“How should this behave if the API call fails?”;把“This is inefficient.”改为“I see this loops through all users. Have we considered the performance impact with 100k users?”。
- 用建议而非命令的协作语气:把“You must change this to use async/await”改为“Suggestion: async/await might make this more readable: `async function fetchUser(id: string) { const user = await db.query('SELECT * FROM users WHERE id = ?', id); return user; }` What do you think?”;把“Extract this into a function”改为“This logic appears in 3 places. Would it make sense to extract it into a shared utility function?”。
- 用标签区分严重程度:🔴 [blocking] 合并前必须修复;🟡 [important] 应修复,有异议可讨论;🟢 [nit] 可选不阻塞;💡 [suggestion] 可考虑的替代方案;📚 [learning] 教育性评论无需行动;🎉 [praise] 做得好。例如“🔴 [blocking] This SQL query is vulnerable to injection. Please use parameterized queries.”“🟢 [nit] Consider renaming `data` to `userData` for clarity.”“🎉 [praise] Excellent test coverage! This will catch edge cases.”。
- Python 专项检查:避免可变默认参数(`def add_item(item, items=[])` 改为 `items=None` 再初始化);避免过宽 except(`except:` 改为 `except ValueError as e:` 并记录日志后 raise);避免可变类属性(`permissions = []` 改为在 `__init__` 中初始化)。
- TypeScript/JavaScript 专项检查:避免 `any`(用 `interface DataPayload { value: string; }` 等具体类型);处理异步错误(检查 `response.ok`、try/catch 并重新抛出);不要修改 props(用 `useEffect` 通知父组件更新)。
- 架构评审模式:大型功能先要设计文档并在实现前与团队评审;分阶段评审(第一个 PR 核心抽象与接口、第二个 PR 实现、第三个 PR 集成与测试);考虑替代方案(“Have we considered using [pattern/library]?”“What's the tradeoff vs. the simpler approach?”“How will this evolve as requirements change?”)。
- 测试质量评审:避免测试实现细节(如断言 `component.state.counter`),改为测试行为(如断言 `screen.getByText('Count: 1')`);评审时问:测试是否描述行为而非实现?命名是否清晰?是否覆盖边界?是否独立无共享状态?能否任意顺序运行?
- 安全评审清单:认证与授权(需要处是否要求认证、每个操作前是否检查授权、JWT 校验是否完整、API key/密钥是否妥善保护);输入校验(所有用户输入是否校验、文件上传是否限制大小与类型、SQL 是否参数化、XSS 是否转义输出);数据保护(密码是否用 bcrypt/argon2 哈希、敏感数据是否加密存储、是否强制 HTTPS、PII 是否合规);常见漏洞(无 eval() 等动态执行、无硬编码密钥、状态变更操作有 CSRF 防护、公开端点有限流)。
- 给出困难反馈时用“上下文 + 具体问题 + 有帮助的解决方案”代替传统三明治法。例如:“I noticed the payment processing logic is inline in the controller. This makes it harder to test and reuse. The calculateTotal() function mixes tax calculation, discount logic, and database queries, making it difficult to unit test and reason about. Could we extract this into a PaymentService class? That would make it testable and reusable. I can pair with you on this if helpful.”。
- 处理分歧:先理解对方(“Help me understand your approach. What led you to choose this pattern?”);承认合理点(“That's a good point about X. I hadn't considered that.”);用数据说话(“I'm concerned about performance. Can we add a benchmark to validate the approach?”);必要时升级(“Let's get [architect/senior dev] to weigh in on this.”);知道何时放手——如果可用且非关键问题就批准,完美是进步的敌人。
- 遵循最佳实践:24 小时内评审(最好当天);限制 PR 大小在 200-400 行;按时间块评审(最多 60 分钟,中间休息);使用 GitHub、GitLab 或专用评审工具;能自动化的就自动化(linter、formatter、安全扫描);用 emoji、表扬和共情建立融洽关系;复杂问题主动提出结对;学习他人的评审评论。
- 避免常见陷阱:完美主义(因小风格偏好阻塞 PR)、范围蔓延(“While you're at it, can you also...”)、不一致(对不同人不同标准)、延迟评审(让 PR 放几天)、消失(要求修改后不见人)、橡皮图章(没真评审就批准)、bike shedding(过度争论琐碎细节)。
- 使用 PR 评审评论模板:Summary(评审概述)、Strengths(做得好的地方)、Required Changes(🔴 阻塞问题)、Suggestions(💡 改进建议)、Questions(❓ 需要澄清或替代方案)、Verdict(✅ 处理必需项后批准)。
适用边界 · 注意事项
- 不要人工评审代码格式、import 组织、lint 违规和简单拼写错误——这些应交给 Prettier、Black 等自动化工具。
- 不要为了炫技、按个人偏好重写、无必要阻塞进度或纠结琐碎细节(bike shedding)。
- 不要在作者有合理解释且问题不关键时坚持己见——可用且非关键就批准,完美是进步的敌人。