code-review-quality

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Code Review Quality

代码评审质量

<default_to_action> When reviewing code or establishing review practices:
  1. PRIORITIZE feedback: 🔴 Blocker (must fix) → 🟡 Major → 🟢 Minor → 💡 Suggestion
  2. FOCUS on: Bugs, security, testability, maintainability (not style preferences)
  3. ASK questions over commands: "Have you considered...?" > "Change this to..."
  4. PROVIDE context: Why this matters, not just what to change
  5. 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> 进行代码评审或建立评审规范时:
  1. 优先处理反馈:🔴 阻塞问题(必须修复)→ 🟡 主要问题 → 🟢 次要问题 → 💡 建议
  2. 重点关注:Bug、安全问题、可测试性、可维护性(而非风格偏好)
  3. 多提问,少命令:使用“你是否考虑过……?”而非“把这个改成……”
  4. 提供上下文:说明修改的原因,而非仅指出要修改的内容
  5. 限制范围:为保证评审效果,每次评审代码不超过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

反馈优先级等级

LevelIconMeaningAction
Blocker🔴Bug/security/crashMust fix before merge
Major🟡Logic issue/test gapShould fix before merge
Minor🟢Style/namingNice to fix
Suggestion💡Alternative approachConsider for future
等级图标含义操作
阻塞问题🔴Bug/安全问题/崩溃合并前必须修复
主要问题🟡逻辑问题/测试缺口合并前建议修复
次要问题🟢风格/命名问题建议修复
建议💡替代方案未来版本可考虑

Review Scope Limits

评审范围限制

Lines ChangedRecommendation
< 200Single review session
200-400Review in chunks
> 400Request PR split
代码变更行数建议
<200单次评审完成
200-400分块评审
>400请求拆分PR

What to Focus On

评审重点与非重点

✅ Review❌ Skip
Logic correctnessFormatting (use linter)
Security risksNaming preferences
Test coverageArchitecture debates
Performance issuesStyle opinions
Error handlingTrivial 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.
undefined
markdown
🔴 **BLOCKER: SQL注入风险**

该查询存在SQL注入漏洞:
```javascript
db.query(`SELECT * FROM users WHERE id = ${userId}`)
修复方案: 使用参数化查询:
javascript
db.query('SELECT * FROM users WHERE id = ?', [userId])
原因: 直接将用户输入嵌入SQL语句会导致攻击者执行任意查询。
undefined

Major (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');
}
undefined
markdown
🟡 **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');
}
undefined

Minor (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 time
aqe/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 mattersJust say "fix this"
Acknowledge good codeOnly point out negatives
Suggest, don't demandBe condescending
Review < 400 linesReview 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可获得一致、快速的初始评审结果。