pr-quality-checklist

PR 质量检查清单技能

概要

全面介绍如何创建高质量的拉取请求，使其易于审查、文档完善并符合团队标准。包含模板、大小指南，以及面向作者和审查者的最佳实践。

适用场景

• 创建任何拉取请求之前
• 审查他人的 PR 时
• 制定团队 PR 标准时
• 新团队成员入职期间
• PR 审查耗时过长时

快速检查清单

创建 PR 之前

• PR 标题遵循约定：type(scope): description
• 描述包含摘要、相关工单和测试计划
• 修改内容聚焦且彼此相关（单一关注点）
• 已自行审查代码，检查明显问题
• 已添加或更新测试，且测试通过
• 没有调试代码（console.log、debugger 等）
• 已解决 TypeScript/类型错误
• 按需更新文档
• UI 修改附带截图
• 已添加变更集（面向用户的修改）

---

PR 标题格式

约定

{type}({scope}): {short description}

类型

• feat：新功能
• fix：修复 Bug
• refactor：既不修复 Bug，也不添加功能的代码修改
• docs：仅修改文档
• style：格式调整、缺少分号等（不修改代码）
• test：添加或更新测试
• chore：维护任务、依赖更新
• perf：性能改进
• ci：CI/CD 修改

示例

feat(auth): add OAuth2 login support
fix(cart): resolve race condition in checkout
refactor(search): extract query param parsing logic
docs(api): update authentication examples
test(payment): add integration tests for Stripe
chore(deps): update Next.js to v15
perf(images): implement lazy loading for gallery
ci(deploy): add staging environment workflow

范围指南

• 使用组件/模块名称：auth、cart、search、api
• 具体但不要过于细化：使用 user-profile，而不是 user-profile-settings-modal
• 与代码库结构保持一致

---

PR 描述模板

标准模板

## Summary

- Added OAuth2 authentication flow
- Integrated with Auth0 provider
- Updated login UI components

## Related Tickets

- Closes #123
- Related to #456
- Fixes ENG-789

## Changes

### Added
- `src/lib/auth/oauth2.ts` - OAuth2 client implementation
- `src/app/api/auth/callback/route.ts` - Auth callback handler
- `src/components/OAuthButtons.tsx` - OAuth login buttons

### Modified
- `src/components/LoginForm.tsx` - Added OAuth buttons
- `src/lib/auth/index.ts` - Export new auth methods
- `README.md` - Updated setup instructions

### Removed
- `src/lib/auth/legacy.ts` - Deprecated auth code
- `src/components/OldLoginForm.tsx` - Replaced by new form

## Screenshots

### Desktop
![Desktop view](https://imgur.com/desktop.png)

### Mobile
![Mobile view](https://imgur.com/mobile.png)

### Before/After (if applicable)
| Before | After |
|--------|-------|
| ![Before](before.png) | ![After](after.png) |

## Testing

- [ ] Unit tests added/updated
- [ ] Integration tests pass
- [ ] Manual testing completed
- [ ] Tested on Chrome, Firefox, Safari
- [ ] Mobile responsive (tested on iOS/Android)
- [ ] Edge cases tested (empty state, error states, loading)

## Breaking Changes

⚠️ **Breaking Change**: The `login()` function signature has changed.

**Before:**
```typescript
login(username: string, password: string)

修改后：

login({ username: string, password: string, provider?: 'local' | 'oauth' })

迁移：

// Old
await login('user', 'pass');

// New
await login({ username: 'user', password: 'pass' });

检查清单

• 代码符合项目风格指南
• 已完成自我审查
• 已为复杂逻辑添加注释
• 已按需更新文档
• 已添加变更集（面向用户的修改）
• 未留下 console.log/debugger 语句
• 没有 TypeScript 错误
• 本地测试通过
• 构建成功

### Minimal Template (for small changes)
```markdown
## Summary
Brief description of the change.

## Changes
- Modified `file.ts` to fix XYZ

## Testing
- [ ] Tests pass
- [ ] Verified manually

---

大小指南

理想的 PR 大小

• 修改行数：< 300（不包括生成的文件）
• 修改文件数：< 15
• 审查时间：< 30 分钟
• 提交数：1-5 个逻辑清晰的提交

大小分类

大小	行数	文件数	审查时间	建议
XS	< 50	1-3	5 min	✅ 热修复的理想选择
S	50-150	3-8	15 min	✅ 大小合适
M	150-300	8-15	30 min	⚠️ 考虑拆分
L	300-500	15-25	1 hour	❌ 应当拆分
XL	500+	25+	2+ hours	❌ 必须拆分

何时拆分 PR

1. 按功能阶段拆分

PR #1: MVP implementation (core functionality)
PR #2: Polish and edge cases
PR #3: Additional features

2. 按层拆分

PR #1: Database schema changes
PR #2: Backend API implementation
PR #3: Frontend UI integration
PR #4: Tests and documentation

3. 按关注点拆分

PR #1: Refactoring (no behavior change)
PR #2: New feature (builds on refactored code)
PR #3: Tests for new feature

4. 按风险级别拆分

PR #1: High-risk changes (need careful review)
PR #2: Low-risk changes (routine updates)

大小限制的例外情况

• 生成的代码（迁移、API 客户端、类型）
• 重命名/移动文件（使用 git mv 展示）
• 批量格式调整（单独提交 PR，并事先获批）
• 第三方库集成（文档完善）

---

重构 PR

重构模板

## Summary
Refactoring and cleanup for [area]

**Goal**: Improve code maintainability without changing behavior

## Motivation
- Reduce code duplication (3 similar functions → 1 reusable utility)
- Improve type safety (any → specific types)
- Remove dead code identified during feature work

## Related Tickets
- ENG-XXX: Improve [area] maintainability
- ENG-YYY: Remove deprecated [feature]

## Stats
- **Lines added**: +91
- **Lines removed**: -1,330
- **Net**: -1,239 ✅
- **Files changed**: 23

## Changes

### Removed (Dead Code)
- `/api/old-endpoint` - unused, replaced by `/api/new-endpoint` in v2.0
- `useDeprecatedHook.ts` - replaced by `useNewHook.ts` (ENG-234)
- `legacy-utils.ts` - functions no longer called anywhere

### Refactored
- **Extracted common logic**: Query param parsing now in `lib/url-utils.ts`
- **Consolidated validation**: 3 duplicate Zod schemas → 1 shared schema
- **Improved types**: Replaced 12 `any` types with proper interfaces

### No Behavior Changes
- [ ] All tests pass (no test modifications needed)
- [ ] Same inputs → same outputs
- [ ] No user-facing changes

## Testing
- [ ] Full test suite passes (no new tests needed)
- [ ] Manual smoke test completed
- [ ] No regressions identified

## Review Focus
- Verify removed code is truly unused (checked with `rg` search)
- Confirm refactored logic is equivalent
- Check no subtle behavior changes introduced

重构最佳实践

• 将重构与功能开发分开：绝不混在一起
• 确认行为没有任何变化：测试不应需要更新
• 说明已删除代码的原因：解释为何可以安全删除
• 彻底搜索：使用 rg/grep 确认没有相关用法
• 展示代码行数减少：突出代码精简成果

---

审查检查清单

面向作者（自我审查）

请求审查之前，请检查：

代码质量

• 代码遵循 DRY 原则（没有明显重复）
• 函数用途单一且目标明确
• 变量名称清晰且具有描述性
• 没有魔法数字或硬编码值
• 错误处理全面
• 已处理边界情况

测试

• 测试覆盖新功能
• 测试有实际意义（不只是为了提高覆盖率）
• 已测试边界情况
• 已测试错误条件
• 没有引入不稳定的测试

文档

• 复杂逻辑有注释
• 公共 API 有 JSDoc/docstrings
• README 已更新（如果设置有变更）
• 提供迁移指南（如果有不兼容变更）

性能

• 没有明显的性能问题
• 数据库查询高效（已考虑索引）
• 没有 N+1 查询
• 已妥善处理大型数据集

安全

• 没有硬编码的密钥
• 已进行输入验证
• 已设置授权检查
• 没有 SQL 注入漏洞
• 不记录敏感数据

---

面向审查者

初步审查（5 分钟）

• PR 描述清晰
• 变更符合所述目标
• 规模合理（最好少于 300 LOC）
• 没有明显的危险信号

代码审查（15-30 分钟）

• 逻辑正确
• 已处理边缘情况
• 错误处理恰当
• 代码易于阅读和维护
• 没有性能问题
• 已考虑安全因素

测试审查

• 测试覆盖了这些变更
• 测试有意义
• 没有不稳定的测试
• 已测试边缘情况

文档审查

• PR 描述与变更一致
• 注释解释“为什么”，而不是“是什么”
• 已记录不兼容变更
• 按需提供迁移指南

批准标准

✅ Approve: Code is good, minor nits only
💬 Comment: Suggestions but not blockers
🔄 Request Changes: Issues must be fixed before merge

---

常见错误

1. PR 标题含糊

❌ "Fix bug"
❌ "Updates"
❌ "WIP"
❌ "Changes from code review"

✅ "fix(auth): prevent duplicate login requests"
✅ "feat(cart): add coupon code support"

2. 缺少上下文

❌ "Changed the function"
Why? What was wrong? What's the impact?

✅ "Refactored parseQueryParams to handle nested objects

Previously failed when query params contained dots (e.g., user.name).
Now correctly parses nested structures using qs library.

Fixes ENG-456"

3. 规模过大

❌ 2,000 lines changed across 50 files
❌ "Implement entire authentication system"

✅ Split into:
  - PR #1: Add database schema for auth (150 LOC)
  - PR #2: Implement JWT utilities (100 LOC)
  - PR #3: Create login endpoint (200 LOC)
  - PR #4: Add login UI (250 LOC)

4. 混杂不同关注点

❌ Single PR with:
  - Feature implementation
  - Dependency updates
  - Refactoring
  - Bug fixes
  - Formatting changes

✅ Separate PRs:
  - PR #1: feat(feature)
  - PR #2: chore(deps)
  - PR #3: refactor(component)

5. UI 变更没有截图

❌ "Updated the header design"
(No screenshots)

✅ "Updated the header design"
[Desktop screenshot]
[Mobile screenshot]
[Before/After comparison]

6. 测试说明不完整

❌ "Tested locally"
How? What scenarios? What browsers?

✅ "Testing completed:
- Unit tests: All 47 tests pass
- Manual testing: Tested login flow on Chrome, Firefox, Safari
- Edge cases: Tested expired tokens, invalid credentials, network errors
- Mobile: Verified on iOS Safari and Android Chrome"

7. 遗留调试代码

❌ console.log('user data:', user);
❌ debugger;
❌ // TODO: fix this later
❌ import { test } from './test-utils'; (unused)

✅ Clean code without debugging artifacts

8. 没有 Changeset（面向用户的变更）

❌ New feature shipped without changelog entry

✅ .changeset/new-feature.md:
---
"@myapp/web": minor
---

Add OAuth2 login support with Auth0 integration

---

总结

快速参考

PR 标题公式

{type}({scope}): {clear, concise description}

理想 PR 的特点

• 规模：少于 300 LOC，少于 15 个文件
• 重点：聚焦单一关注点，仅包含相关变更
• 文档：描述清晰，包含测试计划；如涉及 UI，提供截图
• 质量：已自行审查并测试，不含调试代码
• 时间安排：已准备好合并，而非 WIP

审查心态

作为作者：

• 让审查者的工作更轻松
• 提供上下文和理由
• 请求审查前充分测试
• 及时回应反馈

作为审查者：

• 提供有建设性且具体的意见
• 关注正确性和可维护性
• 有不清楚之处时提出问题
• 肯定良好的实践

危险信号

• 🚩 没有描述，或标题为“WIP”
• 🚩 变更超过 500 LOC
• 🚩 混杂不同关注点（功能 + 重构 + 依赖项）
• 🚩 没有测试，或只修改了测试
• 🚩 “相信我，它能用”
• 🚩 Console.log/debugger 语句
• 🚩 忽略 TypeScript 错误
• 🚩 UI 变更没有截图

使用此技能来维持高标准的 PR 质量，并简化代码审查流程。