code-review-excellence

代码评审卓越实践

通过建设性的反馈、系统化分析和协作改进，让代码评审从把关转变为知识分享。

何时使用此技能

• 评审拉取请求和代码变更
• 为团队制定代码评审标准
• 在评审中指导初级开发人员
• 开展架构评审
• 制定评审清单和指南
• 改进团队协作
• 缩短代码评审周期
• 维护代码质量标准

核心原则

1. 评审心态

代码评审的目标：

• 发现错误和边界情况
• 确保代码可维护
• 在团队内分享知识
• 落实编码标准
• 改进设计和架构
• 建设团队文化

不是代码评审的目标：

• 炫耀知识
• 吹毛求疵地纠结格式（使用代码检查工具）
• 无谓地阻碍进度
• 按照自己的偏好重写代码

2. 有效反馈

良好的反馈应当：

• 具体且可执行
• 具有教育意义，而非带有评判性
• 聚焦代码，而非个人
• 兼顾优缺点（也要肯定好的工作）
• 区分优先级（关键问题与锦上添花的建议）

❌ Bad: "This is wrong."
✅ Good: "This could cause a race condition when multiple users
         access simultaneously. Consider using a mutex here."

❌ Bad: "Why didn't you use X pattern?"
✅ Good: "Have you considered the Repository pattern? It would
         make this easier to test. Here's an example: [link]"

❌ Bad: "Rename this variable."
✅ Good: "[nit] Consider `userCount` instead of `uc` for
         clarity. Not blocking if you prefer to keep it."

3. 评审范围

评审内容：

• 逻辑正确性和边界情况
• 安全漏洞
• 对性能的影响
• 测试覆盖率和质量
• 错误处理
• 文档和注释
• API 设计和命名
• 与架构的契合度

无需人工评审的内容：

• 代码格式（使用 Prettier、Black 等工具）
• 导入项排序
• 代码检查违规
• 简单的拼写错误

评审流程

阶段 1：收集背景信息（2-3 分钟）

开始查看代码前，先了解以下内容：

1. 阅读 PR 描述和关联的问题
2. 查看 PR 的大小（超过 400 行？请对方拆分）
3. 查看 CI/CD 状态（测试是否通过？）
4. 了解业务需求
5. 留意相关的架构决策

阶段 2：高层次评审（5-10 分钟）

1. 架构与设计——解决方案是否符合问题需求？
   • 对于重大变更，请参阅架构评审指南
   • 检查：SOLID 原则、耦合与内聚、反模式
2. 性能评估——是否存在性能方面的顾虑？
   • 对于性能关键型代码，请参阅性能评审指南
   • 检查：算法复杂度、N+1 查询、内存使用情况
3. 文件组织——新文件是否放在了正确的位置？
4. 测试策略——是否有覆盖边界情况的测试？

阶段 3：逐行评审（10-20 分钟）

检查每个文件：

• 逻辑与正确性——边界情况、差一错误、空值检查、竞态条件
• 安全性——输入验证、注入风险、XSS、敏感数据
• 性能——N+1 查询、不必要的循环、内存泄漏
• 可维护性——命名清晰、单一职责、注释
• 复用——接受新代码前，先搜索是否有现成的实用工具或辅助函数可以替代。检查相邻文件和共享模块中是否有类似模式。有关参数泛滥、泄漏式抽象、嵌套条件、字符串化类型代码、TOCTOU 和无操作更新等反模式，请参阅通用质量指南。

阶段 4：总结与决策（2-3 分钟）

1. 总结主要问题
2. 指出值得肯定之处
3. 明确给出决定：
   • ✅ 批准
   • 💬 留言（提出小建议）
   • 🔄 请求修改（必须处理）
4. 如果问题复杂，提出结对处理

评审技巧

技巧 1：清单法

使用清单确保评审一致。全面的安全检查清单请参阅安全评审指南。

技巧 2：提问法

不要直接指出问题，而是提出问题：

❌ "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?"

技巧 3：提出建议，而非发号施令

使用协作式措辞：

❌ "You must change this to use async/await"
✅ "Suggestion: async/await might make this more readable. What do you think?"

❌ "Extract this into a function"
✅ "This logic appears in 3 places. Would it make sense to extract it?"

技巧 4：区分严重程度

使用标签标明优先级：

• 🔴 [blocking] - 合并前必须修复
• 🟡 [important] - 应当修复；若有异议，请讨论
• 🟢 [nit] - 有益但非阻塞
• 💡 [suggestion] - 可考虑的替代方案
• 📚 [learning] - 教育性评论，无需采取行动
• 🎉 [praise] - 做得好，请继续保持！

特定语言指南

根据审查的代码语言，查阅对应的详细指南：

语言/框架	参考文件	主要主题
React	React 指南	Hooks、useEffect、React 19 Actions、RSC、Suspense、TanStack Query v5
Vue 3	Vue 指南	Composition API、响应性系统、Props/Emits、Watchers、Composables
Angular 17+	Angular 指南	Signals、Standalone 组件、RxJS、Zoneless 变更检测、模板优化
Rust	Rust 指南	所有权/借用、Unsafe 审查、异步代码、取消安全性、错误处理
TypeScript	TypeScript 指南	类型安全、async/await、不可变性
Python	Python 指南	可变默认参数、异常处理、类属性
Django / DRF	Django 指南	安全审查、N+1 查询、Serializer 反模式、ViewSet、异步视图
FastAPI	FastAPI 指南	Depends、Pydantic v2 验证、异步正确性、会话/N+1、身份验证与授权、测试驱动验证
Java	Java 指南	Java 17/21 新特性、Spring Boot 3、虚拟线程、Stream/Optional
PHP	PHP 指南	PHP 8.x 类型系统、PDO、安全审查、Composer、PHPUnit/PHPStan
C# / .NET	C# 指南	C# 12 特性、异步编程、EF Core 性能、ASP.NET Core、LINQ
Go	Go 指南	错误处理、goroutine/channel、context、接口设计
Kotlin / Android	Kotlin 指南	协程、Flow、Jetpack Compose、空安全、内存泄漏、架构模式
NestJS	NestJS 指南	依赖注入、分层架构、DTO 验证、Guard/Interceptor、循环依赖
Svelte / SvelteKit	Svelte 指南	Runes、Load 函数、Form Actions、Store 迁移、SSR/CSR 边界
C	C 指南	指针/缓冲区、内存安全、UB、错误处理
C++	C++ 指南	RAII、生命周期、Rule of 0/3/5、异常安全
CSS/Less/Sass	CSS 指南	变量规范、!important、性能优化、响应式、兼容性
Qt	Qt 指南	对象模型、信号/槽、内存管理、线程安全、性能

跨领域指南

适用于所有代码审查的语言无关模式：

主题	参考文件	主要主题
通用质量	通用质量指南	复用审查、参数泛滥、泄漏的抽象、嵌套条件、字符串类型代码、TOCTOU、无操作更新、冗余状态

其他资源

• 架构审查指南 - 架构设计审查指南（SOLID、反模式、耦合度）
• 性能审查指南 - 性能审查指南（Web Vitals、N+1、复杂度）
• 常见错误检查清单 - 按语言分类的常见错误清单
• 安全审查指南 - 安全审查指南
• 代码审查最佳实践 - 代码审查最佳实践
• PR 审查模板 - PR 审查评论模板
• 审查清单 - 快速参考清单