Back to Details

code-review-checklist

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Code Review Checklist

代码评审检查清单

Review Philosophy

评审理念

A code review is a collaborative conversation, not a gatekeeping exercise. The goals are:
  1. Catch bugs before they reach production.
  2. Share knowledge across the team.
  3. Maintain consistency in style and architecture.
  4. Improve the code incrementally.
Review the code, not the author. Be specific, be kind, and suggest alternatives rather than only pointing out problems.
代码评审是协作式的沟通，而非把关式的流程。目标如下：
  1. 在代码上线前发现bug。
  2. 在团队内共享知识。
  3. 保持风格与架构的一致性。
  4. 逐步改进代码质量。
评审的是代码，而非开发者本人。给出具体意见，保持友善，多提出替代方案而非仅指出问题。

Severity Classification

严重程度分类

Classify every comment to set expectations:
SeverityMeaningAction Required
BlockerBug, security flaw, data loss risk, or broken build.Must fix before merge.
MajorDesign issue, missing test, or significant maintainability concern.Should fix before merge.
MinorStyle inconsistency, naming nit, or small improvement.Fix if convenient; OK to defer.
SuggestionAlternative approach, future improvement idea, or "did you consider...?"No action required.
Prefix comments with the severity: 
[Blocker]
, 
[Major]
, 
[Minor]
, 
[Suggestion]
.
为每条评论划分严重程度，明确预期要求：
严重程度含义所需操作
阻塞（Blocker）存在bug、安全漏洞、数据丢失风险或构建失败。合并前必须修复。
主要问题（Major）设计问题、缺少测试或重大可维护性问题。合并前应修复。
次要问题（Minor）风格不一致、命名细节问题或小的改进点。方便时修复；可延后。
建议（Suggestion）替代方案、未来改进想法或“你是否考虑过...？”无需强制修复。
在评论前添加严重程度前缀：
[Blocker]
、
[Major]
、
[Minor]
、
[Suggestion]
。

1. Correctness Checklist

1. 正确性检查表

Does the code do what it is supposed to do?
  • Requirements met. Does the change implement the described feature or fix the described bug completely?
  • Edge cases handled. What happens with null, empty, zero, negative, very large, or unexpected inputs?
  • Error handling. Are exceptions caught at the right level? Are error messages helpful? Do errors propagate correctly?
  • Return values. Are all code paths returning the correct type and value? Are there missing returns?
  • Off-by-one. Are loop bounds, array indices, and string slices correct? Inclusive vs. exclusive?
  • Type safety. Are types correct? Are there implicit coercions that could cause surprises (e.g., 
    "1" + 1
    )?
  • Concurrency. If concurrent, are shared resources protected? Are there potential race conditions or deadlocks?
  • Data integrity. Are database transactions used where needed? Is there a risk of partial writes?
  • Backward compatibility. Does this change break existing callers, APIs, or stored data?
  • Idempotency. If the operation is retried, does it produce the same result?
代码是否实现了预期功能？
  • 满足需求：变更是否完整实现了描述的功能或修复了描述的bug？
  • 处理边界情况：当输入为null、空值、零、负数、超大值或非预期值时，代码表现如何？
  • 错误处理：异常是否在合适的层级被捕获？错误提示是否清晰有用？错误是否正确传播？
  • 返回值：所有代码路径是否返回了正确的类型和值？是否存在遗漏的返回逻辑？
  • 边界索引问题：循环范围、数组索引和字符串切片是否正确？是包含式还是排除式？
  • 类型安全：类型是否正确？是否存在可能导致意外结果的隐式类型转换（如
    "1" + 1
    ）？
  • 并发处理：若涉及并发，共享资源是否受到保护？是否存在潜在的竞态条件或死锁？
  • 数据完整性：是否在需要的地方使用了数据库事务？是否存在部分写入的风险？
  • 向后兼容性：该变更是否会破坏现有的调用方、API或存储的数据？
  • 幂等性：若操作被重试，是否会产生相同的结果？

2. Readability Checklist

2. 可读性检查表

Can a new team member understand this code without asking the author?
  • Naming. Do variable, function, and class names clearly express their purpose? Avoid abbreviations unless universally understood.
  • Function length. Are functions short enough to understand at a glance (under 20-30 lines)? If not, can they be decomposed?
  • Single responsibility. Does each function do one thing? Does each class have one reason to change?
  • Comments. Are comments explaining WHY, not WHAT? Is there dead commented-out code that should be removed?
  • Complexity. Are nested conditionals minimized? Can guard clauses replace deep nesting?
  • Consistency. Does the code follow existing patterns and conventions in the codebase?
  • Magic values. Are there unexplained numbers or strings? Should they be named constants?
  • Code flow. Can you follow the execution path without jumping around? Is control flow straightforward?
  • Formatting. Is indentation, spacing, and line length consistent with the project style?
  • Dead code. Is there unreachable code, unused imports, or unused variables?
新团队成员无需询问作者就能理解这段代码吗？
  • 命名规范：变量、函数和类的名称是否清晰表达了其用途？除非是通用缩写，否则避免使用缩写。
  • 函数长度：函数是否足够简短（20-30行以内），便于一眼理解？如果不是，能否拆分？
  • 单一职责：每个函数是否只做一件事？每个类是否只有一个变更理由？
  • 注释规范：注释是否解释了“为什么”而非“是什么”？是否存在应删除的已注释掉的无效代码？
  • 复杂度控制：嵌套条件是否已最小化？能否用卫语句替代深层嵌套？
  • 一致性：代码是否遵循代码库中已有的模式和约定？
  • 魔法值：是否存在未解释的数字或字符串？是否应将其定义为命名常量？
  • 代码流程：能否不跳转就跟上执行路径？控制流是否直观？
  • 格式规范：缩进、空格和行长度是否与项目风格一致？
  • 无效代码：是否存在不可达代码、未使用的导入或未使用的变量？

3. Maintainability Checklist

3. 可维护性检查表

Will this code be easy to change six months from now?
  • DRY (Don't Repeat Yourself). Is there duplicated logic that should be extracted into a shared function?
  • Coupling. Is the code tightly coupled to specific implementations? Could dependency injection improve flexibility?
  • Abstraction level. Are high-level and low-level concerns mixed? (e.g., business logic mixed with SQL queries or HTTP handling)
  • Configuration. Are environment-specific values (URLs, timeouts, thresholds) configurable rather than hardcoded?
  • Module boundaries. Does the change respect existing module/package boundaries? Does it introduce circular dependencies?
  • API design. Are public interfaces minimal and well-defined? Can internal details change without affecting callers?
  • Migration path. If this changes a schema, API, or data format, is there a migration plan?
  • Documentation. Are public APIs documented? Are complex algorithms explained?
  • Deprecation. If replacing old code, is the old code marked as deprecated with a removal timeline?
  • Feature flags. For risky or incremental changes, should the new behavior be behind a feature flag?
六个月后，这段代码是否容易修改？
  • DRY原则（避免重复）：是否存在应提取到共享函数中的重复逻辑？
  • 耦合度：代码是否与特定实现紧密耦合？依赖注入能否提升灵活性？
  • 抽象层级：是否混合了高层和低层关注点？（例如，业务逻辑与SQL查询或HTTP处理混合）
  • 配置管理：环境特定的值（URL、超时时间、阈值）是否可配置，而非硬编码？
  • 模块边界：变更是否尊重现有的模块/包边界？是否引入了循环依赖？
  • API设计：公共接口是否简洁且定义清晰？内部细节变更是否不会影响调用方？
  • 迁移方案：如果变更了 schema、API 或数据格式，是否有迁移计划？
  • 文档完善：公共API是否有文档？复杂算法是否有说明？
  • 废弃标记：如果替换旧代码，是否已将旧代码标记为废弃并给出移除时间线？
  • 功能开关：对于有风险或增量式的变更，是否应将新行为放在功能开关后？

4. Security Checklist

4. 安全性检查表

Could this code be exploited by a malicious actor?
  • Input validation. Is all user input validated and sanitized before use?
  • SQL injection. Are database queries parameterized? No string concatenation for SQL.
  • XSS (Cross-Site Scripting). Is user-generated content escaped before rendering in HTML?
  • CSRF (Cross-Site Request Forgery). Are state-changing endpoints protected with CSRF tokens?
  • Authentication. Are protected routes/resources properly gated behind authentication?
  • Authorization. Does the code verify the user has permission to access/modify the specific resource?
  • Secrets. Are API keys, passwords, or tokens hardcoded? They should be in environment variables or a secrets manager.
  • Logging. Are sensitive data (passwords, tokens, PII) excluded from logs?
  • Dependency vulnerabilities. Are new dependencies from trusted sources? Have they been audited?
  • File handling. Are uploaded files validated for type and size? Are paths sanitized to prevent directory traversal?
  • Rate limiting. Are public-facing endpoints protected against abuse?
  • HTTPS. Are external API calls using HTTPS? Are certificates validated?
恶意攻击者能否利用这段代码？
  • 输入验证：所有用户输入在使用前是否经过验证和清理？
  • SQL注入防护：数据库查询是否使用参数化？是否存在通过字符串拼接构造SQL的情况？
  • XSS（跨站脚本）防护：用户生成的内容在HTML中渲染前是否经过转义？
  • CSRF（跨站请求伪造）防护：状态变更的接口是否通过CSRF令牌进行保护？
  • 身份验证：受保护的路由/资源是否正确通过身份验证进行管控？
  • 授权验证：代码是否验证了用户是否有权访问/修改特定资源？
  • 密钥管理：API密钥、密码或令牌是否被硬编码？它们应存储在环境变量或密钥管理工具中。
  • 日志规范：日志中是否排除了敏感数据（密码、令牌、个人可识别信息）？
  • 依赖漏洞：新引入的依赖是否来自可信源？是否经过审计？
  • 文件处理：上传的文件是否经过类型和大小验证？路径是否经过清理以防止目录遍历攻击？
  • 限流防护：面向公网的接口是否有防滥用的限流措施？
  • HTTPS使用：外部API调用是否使用HTTPS？证书是否经过验证？

5. Performance Checklist

5. 性能检查表

Will this code perform acceptably under expected load?
  • Algorithmic complexity. Is the algorithm appropriate for the data size? Is O(n^2) acceptable here?
  • Database queries. Are queries indexed? Are there N+1 query problems? Are results paginated?
  • Caching. Should results be cached? If caching exists, is invalidation handled correctly?
  • Payload size. Are API responses, database fetches, or file reads bounded in size?
  • Lazy loading. Are expensive resources loaded only when needed?
  • Connection pooling. Are database/HTTP connections pooled and reused, not created per request?
  • Async / non-blocking. Are I/O operations non-blocking where appropriate? Are there unnecessary 
    await
    s on non-dependent operations?
  • Memory. Are large data sets streamed rather than loaded entirely into memory?
  • Startup impact. Does this change slow down application startup?
  • Monitoring. Are new critical code paths instrumented with metrics or tracing?
在预期负载下，这段代码的性能是否可接受？
  • 算法复杂度：算法是否适合当前的数据规模？O(n^2)的复杂度是否可接受？
  • 数据库查询：查询是否有索引？是否存在N+1查询问题？结果是否分页？
  • 缓存策略：结果是否应该被缓存？如果已有缓存，缓存失效逻辑是否正确？
  • 负载大小：API响应、数据库查询或文件读取的大小是否有上限？
  • 懒加载：昂贵的资源是否仅在需要时才加载？
  • 连接池：数据库/HTTP连接是否使用连接池复用，而非每个请求创建新连接？
  • 异步/非阻塞：I/O操作是否在合适的场景下使用非阻塞模式？是否在不相关的操作上存在不必要的
    await
    ？
  • 内存使用：大数据集是否采用流式处理，而非全部加载到内存中？
  • 启动影响：该变更是否会减慢应用启动速度？
  • 监控埋点：新的关键代码路径是否添加了指标或链路追踪？

6. Testing Checklist

6. 测试检查表

Is the change adequately tested?
  • Tests exist. Does every new behavior have at least one test?
  • Tests pass. Do all tests pass in CI? Have you run them locally?
  • Happy path tested. Are the primary use cases covered?
  • Edge cases tested. Are boundary values, empty inputs, and error conditions tested?
  • Regression test. If this is a bug fix, is there a test that would have caught the bug?
  • Test quality. Do tests assert on behavior, not implementation details?
  • Test isolation. Do tests run independently? No shared mutable state between tests?
  • Test readability. Can you understand what each test verifies from its name and body?
  • Mocking. Are mocks used judiciously? Are they mocking the right boundaries (I/O, not business logic)?
  • Coverage. Is coverage maintained or improved? Are critical paths covered?
  • Flakiness. Do new tests depend on timing, external services, or non-deterministic data?
变更是否得到了充分的测试？
  • 存在测试：每个新行为是否至少有一个对应的测试？
  • 测试通过：所有测试在CI中是否通过？你是否在本地运行过测试？
  • 主流程测试：主要使用场景是否被覆盖？
  • 边界情况测试：边界值、空输入和错误场景是否被测试？
  • 回归测试：如果是bug修复，是否有能捕获该bug的测试？
  • 测试质量：测试是否断言行为而非实现细节？
  • 测试隔离：测试是否独立运行？测试之间是否没有共享的可变状态？
  • 测试可读性：从测试的名称和内容能否理解它要验证的内容？
  • Mock使用：Mock是否被合理使用？是否在正确的边界（I/O而非业务逻辑）上使用Mock？
  • 测试覆盖率：覆盖率是否保持或提升？关键路径是否被覆盖？
  • 测试稳定性：新测试是否依赖于时序、外部服务或非确定性数据？

How to Give Constructive Feedback

如何给出建设性反馈

Do

建议做法

  • Be specific. Point to exact lines. Explain the problem concretely.
  • Explain why. Not just "change this" but "this could cause X because Y."
  • Suggest alternatives. Provide a code snippet or link to a pattern.
  • Ask questions. "What happens if X is null here?" is better than "You forgot a null check."
  • Acknowledge good work. Call out clever solutions, clean refactoring, or good test coverage.
  • Distinguish severity. A nit is not a blocker. Label them differently.
  • Offer to pair. If the change is complex, offer to discuss live.
  • 具体明确：指向具体代码行，具体解释问题。
  • 说明原因：不只是“修改这个”，而是“这可能导致X问题，因为Y”。
  • 给出替代方案：提供代码片段或模式链接。
  • 提出问题：“如果X为null会发生什么？”比“你忘了做空值检查”更好。
  • 认可优秀工作：指出巧妙的解决方案、清晰的重构或完善的测试覆盖。
  • 区分严重程度：细节问题不是阻塞问题，要区分标记。
  • 提供结对支持：如果变更复杂，主动提出实时讨论。

Do Not

避免做法

  • Do not attack the person. "This code is bad" vs. "This function could be simplified by..."
  • Do not bikeshed. Do not block a PR over trivial style preferences that are not in the style guide.
  • Do not pile on. If another reviewer already made a point, a thumbs-up is enough.
  • Do not be vague. "This is confusing" is not actionable. Say what is confusing and why.
  • Do not rewrite. If you would rewrite the entire approach, have a conversation first — do not do it through line comments.
  • 不攻击个人：不说“这段代码很差”，而是“这个函数可以通过...来简化”。
  • 不纠结小事：不要因为风格指南中没有规定的琐碎偏好而阻塞PR。
  • 不重复评论：如果其他评审者已经提出了相同观点，点赞即可。
  • 不模糊表述：“这很混乱”没有可操作性，要说明哪里混乱以及原因。
  • 不直接重写：如果你想完全重写实现，先进行沟通，不要通过行内评论直接修改。

Comment Templates

评论模板

[Blocker] Bug: `items.length` will throw if `items` is null.
Consider adding a null check: `if (!items?.length) return [];`

[Major] Missing test: This new validation logic does not have
a corresponding test for the rejection case.

[Minor] Naming: `d` is ambiguous. Consider `durationMs` or
`elapsedDays` to clarify the unit.

[Suggestion] You could use `Array.flatMap()` here to combine
the map and filter into one pass. Not required though.
[Blocker] Bug: `items.length` 在 `items` 为null时会抛出错误。
建议添加空值检查：`if (!items?.length) return [];`

[Major] 缺少测试：这个新的验证逻辑没有对应的拒绝场景测试。

[Minor] 命名问题：`d` 含义模糊，建议改为 `durationMs` 或
`elapsedDays` 以明确单位。

[Suggestion] 你可以在这里使用 `Array.flatMap()`，将map和filter合并为一次遍历。非强制要求。

Reviewer Responsibilities

评审者职责

  • Respond to review requests within one business day.
  • Review the full diff, not just the files you are familiar with.
  • Pull the branch and run it locally for non-trivial changes.
  • Check CI results before approving.
  • Re-review after changes are made to your feedback.
  • Approve explicitly when satisfied; do not leave PRs in limbo.
  • 在一个工作日内响应评审请求。
  • 评审完整的代码差异，而非仅你熟悉的文件。
  • 对于非 trivial 的变更，拉取分支并在本地运行测试。
  • 批准前检查CI结果。
  • 针对你提出的反馈，在变更后重新评审。
  • 满意时明确批准；不要让PR处于悬而未决的状态。

Author Responsibilities

提交者职责

  • Self-review your diff before requesting review.
  • Keep PRs small (under 400 lines of meaningful changes).
  • Write a clear PR description explaining what, why, and how.
  • Link to the issue or spec being implemented.
  • Respond to all comments, even if just "Done" or "Acknowledged."
  • Do not merge until all blocker/major comments are resolved.
  • Squash fixup commits before merging if the project convention requires it.
  • 在请求评审前，先自行评审代码差异。
  • 保持PR规模较小（有意义的变更不超过400行）。
  • 编写清晰的PR描述，说明变更内容、原因和实现方式。
  • 关联对应的需求issue或实现规范。
  • 回复所有评论，即使只是“已完成”或“已确认”。
  • 所有阻塞/主要问题解决前，不要合并PR。
  • 如果项目约定要求，合并前压缩修复提交。

Review Workflow

评审工作流程

  1. Skim first. Read the PR description and get the big picture.
  2. Architecture pass. Review the overall design and file structure.
  3. Detailed pass. Go file by file, line by line, applying the checklists above.
  4. Test pass. Read the tests. Do they cover the right cases?
  5. Run it. For anything non-trivial, check out the branch and run it.
  6. Summarize. Leave an overall summary comment with your verdict.
  1. 先快速浏览：阅读PR描述，了解整体情况。
  2. 架构评审：评审整体设计和文件结构。
  3. 详细评审：逐文件、逐行检查，应用上述检查表。
  4. 测试评审：阅读测试用例，检查是否覆盖了正确的场景。
  5. 运行验证：对于非trivial的变更，拉取分支并运行测试。
  6. 总结反馈：留下整体总结评论，给出你的评审结论。