code-review-quality
Compare original and translation side by side
🇺🇸
Original
English🇨🇳
Translation
ChineseCode Review Quality
代码评审质量
<default_to_action>
When reviewing code or establishing review practices:
- PRIORITIZE feedback: 🔴 Blocker (must fix) → 🟡 Major → 🟢 Minor → 💡 Suggestion
- FOCUS on: Bugs, security, testability, maintainability (not style preferences)
- ASK questions over commands: "Have you considered...?" > "Change this to..."
- PROVIDE context: Why this matters, not just what to change
- LIMIT scope: Review < 400 lines at a time for effectiveness
Quick Review Checklist:
- Logic: Does it work correctly? Edge cases handled?
- Security: Input validation? Auth checks? Injection risks?
- Testability: Can this be tested? Is it tested?
- Maintainability: Clear naming? Single responsibility? DRY?
- Performance: O(n²) loops? N+1 queries? Memory leaks?
Critical Success Factors:
- Review the code, not the person
- Catching bugs > nitpicking style
- Fast feedback (< 24h) > thorough feedback </default_to_action>
<default_to_action>
进行代码评审或建立评审规范时:
- 优先处理反馈:🔴 阻塞问题(必须修复)→ 🟡 主要问题 → 🟢 次要问题 → 💡 建议
- 重点关注:Bug、安全问题、可测试性、可维护性(而非风格偏好)
- 多提问,少命令:使用“你是否考虑过……?”而非“把这个改成……”
- 提供上下文:说明修改的原因,而非仅指出要修改的内容
- 限制范围:为保证评审效果,每次评审代码不超过400行
快速评审检查清单:
- 逻辑:代码是否能正常工作?是否处理了边缘情况?
- 安全:是否做了输入验证?是否有身份验证检查?是否存在注入风险?
- 可测试性:这段代码可测试吗?是否已测试?
- 可维护性:命名是否清晰?是否符合单一职责原则?是否遵循DRY原则?
- 性能:是否存在O(n²)循环?是否有N+1查询?是否存在内存泄漏?
关键成功因素:
- 评审代码,而非针对个人
- 发现Bug > 纠结风格细节
- 快速反馈(<24小时)> 过于详尽的反馈 </default_to_action>
Quick Reference Card
快速参考卡片
When to Use
适用场景
- PR code reviews
- Pair programming feedback
- Establishing team review standards
- Mentoring developers
- PR代码评审
- 结对编程反馈
- 建立团队评审标准
- 指导开发人员
Feedback Priority Levels
反馈优先级等级
| Level | Icon | Meaning | Action |
|---|---|---|---|
| Blocker | 🔴 | Bug/security/crash | Must fix before merge |
| Major | 🟡 | Logic issue/test gap | Should fix before merge |
| Minor | 🟢 | Style/naming | Nice to fix |
| Suggestion | 💡 | Alternative approach | Consider for future |
| 等级 | 图标 | 含义 | 操作 |
|---|---|---|---|
| 阻塞问题 | 🔴 | Bug/安全问题/崩溃 | 合并前必须修复 |
| 主要问题 | 🟡 | 逻辑问题/测试缺口 | 合并前建议修复 |
| 次要问题 | 🟢 | 风格/命名问题 | 建议修复 |
| 建议 | 💡 | 替代方案 | 未来版本可考虑 |
Review Scope Limits
评审范围限制
| Lines Changed | Recommendation |
|---|---|
| < 200 | Single review session |
| 200-400 | Review in chunks |
| > 400 | Request PR split |
| 代码变更行数 | 建议 |
|---|---|
| <200 | 单次评审完成 |
| 200-400 | 分块评审 |
| >400 | 请求拆分PR |
What to Focus On
评审重点与非重点
| ✅ Review | ❌ Skip |
|---|---|
| Logic correctness | Formatting (use linter) |
| Security risks | Naming preferences |
| Test coverage | Architecture debates |
| Performance issues | Style opinions |
| Error handling | Trivial changes |
| ✅ 重点评审 | ❌ 无需关注 |
|---|---|
| 逻辑正确性 | 格式问题(使用代码检查工具) |
| 安全风险 | 命名偏好 |
| 测试覆盖率 | 架构争议 |
| 性能问题 | 风格观点 |
| 错误处理 | 微小变更 |
Feedback Templates
反馈模板
Blocker (Must Fix)
阻塞问题(必须修复)
markdown
🔴 **BLOCKER: SQL Injection Risk**
This query is vulnerable to SQL injection:
```javascript
db.query(`SELECT * FROM users WHERE id = ${userId}`)Fix: Use parameterized queries:
javascript
db.query('SELECT * FROM users WHERE id = ?', [userId])Why: User input directly in SQL allows attackers to execute arbitrary queries.
undefinedmarkdown
🔴 **BLOCKER: SQL注入风险**
该查询存在SQL注入漏洞:
```javascript
db.query(`SELECT * FROM users WHERE id = ${userId}`)修复方案: 使用参数化查询:
javascript
db.query('SELECT * FROM users WHERE id = ?', [userId])原因: 直接将用户输入嵌入SQL语句会导致攻击者执行任意查询。
undefinedMajor (Should Fix)
主要问题(建议修复)
markdown
🟡 **MAJOR: Missing Error Handling**
What happens if `fetchUser()` throws? The error bubbles up unhandled.
**Suggestion:** Add try/catch with appropriate error response:
```javascript
try {
const user = await fetchUser(id);
return user;
} catch (error) {
logger.error('Failed to fetch user', { id, error });
throw new NotFoundError('User not found');
}undefinedmarkdown
🟡 **MAJOR: 缺少错误处理**
如果`fetchUser()`抛出异常会发生什么?错误会无处理地向上冒泡。
**建议:** 添加try/catch块并返回合适的错误响应:
```javascript
try {
const user = await fetchUser(id);
return user;
} catch (error) {
logger.error('Failed to fetch user', { id, error });
throw new NotFoundError('User not found');
}undefinedMinor (Nice to Fix)
次要问题(建议修复)
markdown
🟢 **minor:** Variable name could be clearer
`d` doesn't convey meaning. Consider `daysSinceLastLogin`.markdown
🟢 **minor: 变量名称可更清晰**
`d`无法表达含义,建议改为`daysSinceLastLogin`。Suggestion (Consider)
建议(可考虑)
markdown
💡 **suggestion:** Consider extracting this to a helper
This validation logic appears in 3 places. A `validateEmail()` helper would reduce duplication. Not blocking, but might be worth a follow-up PR.markdown
💡 **suggestion: 考虑提取为辅助函数**
该验证逻辑出现在3个地方。创建一个`validateEmail()`辅助函数可减少重复代码。不阻塞合并,但后续PR可考虑优化。Review Questions to Ask
评审提问参考
Logic
逻辑方面
- What happens when X is null/empty/negative?
- Is there a race condition here?
- What if the API call fails?
- 当X为null/空值/负值时会发生什么?
- 这里是否存在竞态条件?
- 如果API调用失败会怎样?
Security
安全方面
- Is user input validated/sanitized?
- Are auth checks in place?
- Any secrets or PII exposed?
- 用户输入是否经过验证/清理?
- 是否有身份验证检查?
- 是否暴露了敏感信息或个人可识别信息(PII)?
Testability
可测试性方面
- How would you test this?
- Are dependencies injectable?
- Is there a test for the happy path? Edge cases?
- 你会如何测试这段代码?
- 依赖是否可注入?
- 是否有针对正常流程和边缘情况的测试?
Maintainability
可维护性方面
- Will the next developer understand this?
- Is this doing too many things?
- Is there duplication we could reduce?
- 下一位开发人员能理解这段代码吗?
- 这段代码是否承担了过多职责?
- 是否存在可减少的重复代码?
Agent-Assisted Reviews
Agent辅助评审
typescript
// Comprehensive code review
await Task("Code Review", {
prNumber: 123,
checks: ['security', 'performance', 'testability', 'maintainability'],
feedbackLevels: ['blocker', 'major', 'minor'],
autoApprove: { maxBlockers: 0, maxMajor: 2 }
}, "qe-quality-analyzer");
// Security-focused review
await Task("Security Review", {
prFiles: changedFiles,
scanTypes: ['injection', 'auth', 'secrets', 'dependencies']
}, "qe-security-scanner");
// Test coverage review
await Task("Coverage Review", {
prNumber: 123,
requireNewTests: true,
minCoverageDelta: 0
}, "qe-coverage-analyzer");typescript
// 全面代码评审
await Task("Code Review", {
prNumber: 123,
checks: ['security', 'performance', 'testability', 'maintainability'],
feedbackLevels: ['blocker', 'major', 'minor'],
autoApprove: { maxBlockers: 0, maxMajor: 2 }
}, "qe-quality-analyzer");
// 安全专项评审
await Task("Security Review", {
prFiles: changedFiles,
scanTypes: ['injection', 'auth', 'secrets', 'dependencies']
}, "qe-security-scanner");
// 测试覆盖率评审
await Task("Coverage Review", {
prNumber: 123,
requireNewTests: true,
minCoverageDelta: 0
}, "qe-coverage-analyzer");Agent Coordination Hints
Agent协作提示
Memory Namespace
内存命名空间
aqe/code-review/
├── review-history/* - Past review decisions
├── patterns/* - Common issues by team/repo
├── feedback-templates/* - Reusable feedback
└── metrics/* - Review turnaround timeaqe/code-review/
├── review-history/* - 过往评审决策
├── patterns/* - 团队/仓库常见问题
├── feedback-templates/* - 可复用反馈模板
└── metrics/* - 评审周转时间Fleet Coordination
集群协作
typescript
const reviewFleet = await FleetManager.coordinate({
strategy: 'code-review',
agents: [
'qe-quality-analyzer', // Logic, maintainability
'qe-security-scanner', // Security risks
'qe-performance-tester', // Performance issues
'qe-coverage-analyzer' // Test coverage
],
topology: 'parallel'
});typescript
const reviewFleet = await FleetManager.coordinate({
strategy: 'code-review',
agents: [
'qe-quality-analyzer', // 逻辑、可维护性
'qe-security-scanner', // 安全风险
'qe-performance-tester', // 性能问题
'qe-coverage-analyzer' // 测试覆盖率
],
topology: 'parallel'
});Review Etiquette
评审礼仪
| ✅ Do | ❌ Don't |
|---|---|
| "Have you considered...?" | "This is wrong" |
| Explain why it matters | Just say "fix this" |
| Acknowledge good code | Only point out negatives |
| Suggest, don't demand | Be condescending |
| Review < 400 lines | Review 2000 lines at once |
| ✅ 建议做法 | ❌ 避免做法 |
|---|---|
| 使用“你是否考虑过……?” | 直接说“这是错的” |
| 说明修改的重要性 | 只说“修复这个” |
| 认可优质代码 | 只指出问题 |
| 提出建议,而非命令 | 态度傲慢 |
| 评审代码不超过400行 | 一次性评审2000行代码 |
Related Skills
相关技能
- agentic-quality-engineering - Agent coordination
- security-testing - Security review depth
- refactoring-patterns - Maintainability patterns
- agentic-quality-engineering - Agent协作
- security-testing - 安全评审深度
- refactoring-patterns - 可维护性模式
Remember
注意事项
Prioritize feedback: 🔴 Blocker → 🟡 Major → 🟢 Minor → 💡 Suggestion. Focus on bugs and security, not style. Ask questions, don't command. Review < 400 lines at a time. Fast feedback (< 24h) beats thorough feedback.
With Agents: Agents automate security, performance, and coverage checks, freeing human reviewers to focus on logic and design. Use agents for consistent, fast initial review.
优先处理反馈: 🔴 阻塞问题 → 🟡 主要问题 → 🟢 次要问题 → 💡 建议。重点关注Bug和安全问题,而非风格。多提问,少命令。每次评审代码不超过400行。快速反馈(<24小时)优于详尽反馈。
使用Agent: Agent可自动完成安全、性能和覆盖率检查,让人工评审者能专注于逻辑和设计。使用Agent可获得一致、快速的初始评审结果。