Back to Details

code-review

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Review Pull Request

Pull Request 审查

Mode: $ARGUMENTS — 
BUGS
, 
SECURITY
, or 
PERFORMANCE
narrows the review to that axis; otherwise apply all.
模式：$ARGUMENTS — 指定
BUGS
、
SECURITY
或
PERFORMANCE
可将审查聚焦于对应维度；未指定则应用全部维度。

Approval standard

批准标准

Approve when the change definitely improves overall code health, even if it isn't perfect. Don't block a change because it isn't how you would have written it. If it improves the codebase and follows its conventions, approve.
If the change is too large to review well (~1000+ lines), asking the author to split it is a valid outcome. Suggest a strategy: stack (small change, next one based on it), by file group, horizontal (shared code first, then consumers), or vertical (one end-to-end slice per PR).
当变更明确提升整体代码健康度时即可批准，即便它并不完美。不要因为变更的实现方式与你自己的写法不同就阻止它。如果它能改进代码库且符合代码库的规范，就批准。
如果变更过大（约1000行以上）难以妥善审查，要求作者拆分是合理的处理方式。可建议以下拆分策略：堆叠式（小变更，后续变更基于此）、按文件组拆分、水平拆分（先提交共享代码，再提交使用方代码）或垂直拆分（每个PR对应一个端到端的功能切片）。

Review criteria

审查要点

Review every changed file, not just the latest commit.
  • Correctness: logic bugs, off-by-ones, race conditions, unhandled states, missing error paths
  • Readability: functions > 50 lines, nesting > 2 levels, dead code, unused imports
  • Security: exposed secrets, unvalidated external data
  • Type safety & immutability: 
    any
    or unjustified assertions; in-place mutation of shared data
  • Patterns: consistent with codebase conventions, no reinvented wheels
  • Performance: unnecessary re-renders, O(n²) where O(n) works
  • Code smells: match the diff against the baseline in smells.md — always judgement calls; the repo's documented style overrides the baseline
  • Dependency upgrades (manifest/lockfile in the diff): one dependency per change — a bulk bump that breaks hides which package did it; verify against the changelog, not the version number; review the lockfile diff (one direct bump pulls dozens of transitive changes); flag hand-edited lockfiles
审查所有已变更的文件，而非仅审查最新提交。
  • 正确性：逻辑漏洞、差一错误、竞态条件、未处理状态、缺失的错误路径
  • 可读性：函数超过50行、嵌套超过2层、死代码、未使用的导入
  • 安全性：密钥泄露、未校验的外部数据
  • 类型安全与不可变性：
    any
    或不合理的类型断言；共享数据的原地修改
  • 代码模式：与代码库规范保持一致，不重复造轮子
  • 性能：不必要的重渲染、本可用O(n)实现却使用O(n²)的方案
  • 代码异味：将diff与smells.md中的基线进行比对——这始终需要主观判断；仓库已文档化的风格规范优先于基线
  • 依赖升级（diff中包含manifest/lockfile）：每次变更仅升级一个依赖——批量升级会掩盖问题来源的包；需对照更新日志验证，而非仅看版本号；审查lockfile的diff（一个直接依赖的升级会带来数十个传递性依赖的变更）；标记手动编辑的lockfile

Output

输出规范

Group by severity, 
file:line
on every finding, a suggested fix for every Critical:
  • Critical — must fix before merge (bugs, vulnerabilities)
  • Suggestions — improvements worth considering
  • Nit — minor and optional; label true nitpicks as Nit rather than dropping or inflating them
  • FYI — informational
  • Positives — good patterns to call out
One structural problem and ten nits: the structural problem is the review — lead with it. Close with the verdict against the approval standard.
按严重程度分组，每条发现都标注
file:line
格式的位置，每个严重问题都必须给出修复建议：
  • Critical — 合并前必须修复（漏洞、安全隐患）
  • Suggestions — 值得考虑的改进项
  • Nit — 次要且可选；将真正的吹毛求疵的问题标记为Nit，不要忽略也不要夸大其严重性
  • FYI — 仅供参考
  • Positives — 值得指出的优秀模式
如果存在一个结构性问题和十个小问题：结构性问题_才是_审查的核心——需放在最前面。最后根据批准标准给出审查结论。