Back to Details

chinese-code-review

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

中文代码审查规范

Chinese Code Review Guidelines

概述

Overview

国内团队做 Code Review 常遇到两个极端：要么过度客气导致关键问题被放过，要么照搬西方直白风格让同事下不来台。本技能帮你找到平衡点——既不回避问题，又让人愿意接受反馈。
核心原则： 用"建议"代替"命令"，用"提问"代替"否定"，但绝不因为面子而放过 bug。
Domestic teams often encounter two extremes when conducting Code Review: either being overly polite and letting critical issues slip through the cracks, or copying the straightforward Western style which makes colleagues feel embarrassed. This guideline helps you find a balance—neither avoiding problems nor making others unwilling to accept feedback.
Core Principles: Replace "commands" with "suggestions", replace "negations" with "questions", but never overlook bugs for the sake of saving face.

审查反馈的表达方式

Expression of Review Feedback

用建议代替命令

Use Suggestions Instead of Commands

避免（命令式）推荐（建议式）
你必须改成 X建议考虑用 X，因为 Y
这里写错了这里可能存在一个问题，是否考虑过 Z 的情况？
不要用这个方法这个方法在 A 场景下可能有性能问题，可以看看 B 方案
这段代码不行这段逻辑我理解得对吗？如果输入为空的话会怎样？
Avoid (Imperative)Recommended (Suggestive)
You must change this to XIt is recommended to consider using X because Y
This is wrongThere might be an issue here. Have you considered scenario Z?
Don't use this methodThis method may have performance issues in scenario A. You can look into solution B
This code is not acceptableDid I understand this logic correctly? What happens if the input is empty?

用提问代替否定

Use Questions Instead of Negations

当你不确定对方意图时，先问再评：
undefined
When you're unsure of the other party's intent, ask first before commenting:
undefined

好的方式

Good practice

这里用 sync 方式读文件是出于什么考虑？如果并发量上来，可能会阻塞事件循环。
What's the consideration for using sync to read files here? If the concurrency increases, it may block the event loop.

不好的方式

Bad practice

这里不应该用 sync 方式读文件。
undefined
You shouldn't use sync to read files here.
undefined

分级标注

Priority Labeling

统一使用优先级标记，让作者快速判断轻重缓急：
  • [必须修复] — 安全漏洞、数据丢失风险、逻辑错误（不修不能合）
  • [建议修改] — 性能问题、可维护性、缺少校验（本次或下次迭代修复）
  • [仅供参考] — 命名优化、风格建议、替代方案（不改也行）
  • [问题] — 不确定的地方，需要作者解释意图
Use unified priority markers to help authors quickly judge urgency:
  • [Must Fix] — Security vulnerabilities, data loss risks, logical errors (cannot merge without fixing)
  • [Suggested Modification] — Performance issues, maintainability, missing validations (can be fixed in this iteration or the next)
  • [For Reference Only] — Naming optimization, style suggestions, alternative solutions (optional to fix)
  • [Question] — Uncertain areas that require the author to explain intent

审查评论模板

Review Comment Template

[必须修复] SQL 注入风险

第 42 行：用户输入直接拼接到 SQL 语句中。

原因：攻击者可以通过 name 参数注入 `'; DROP TABLE users; --`。

建议：使用参数化查询：
  db.query('SELECT * FROM users WHERE name = $1', [name])

参考：https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html
[Must Fix] SQL Injection Risk

Line 42: User input is directly concatenated into the SQL statement.

Reason: Attackers can inject `'; DROP TABLE users; --` via the name parameter.

Suggestion: Use parameterized queries:
  db.query('SELECT * FROM users WHERE name = $1', [name])

Reference: https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html

中英混排代码注释规范

Chinese-English Mixed Code Comment Standards

何时用中文

When to Use Chinese

  • 业务逻辑说明 — 用中文解释业务背景和需求来源
  • 复杂算法注释 — 用中文写思路，确保团队成员都能理解
  • TODO / FIXME — 用中文描述待办事项，方便搜索和追踪
  • 文档注释（内部项目） — JSDoc / Javadoc 中的描述文字用中文
typescript
/**
 * 计算用户的会员等级折扣
 *
 * 业务规则：
 * - 普通会员 9.5 折
 * - 银卡会员 9 折
 * - 金卡会员 8.5 折
 * - 钻石会员 8 折
 *
 * @param level - 会员等级（MemberLevel enum）
 * @param amount - 原始金额（单位：分）
 * @returns 折后金额（单位：分）
 */
function calculateDiscount(level: MemberLevel, amount: number): number {
  // ...
}
  • Business Logic Explanations — Use Chinese to explain business background and demand sources
  • Complex Algorithm Comments — Use Chinese to write the logic to ensure all team members can understand
  • TODO / FIXME — Use Chinese to describe pending tasks for easy search and tracking
  • Document Comments (Internal Projects) — Use Chinese for descriptive text in JSDoc / Javadoc
typescript
/**
 * 计算用户的会员等级折扣
 *
 * 业务规则：
 * - 普通会员 9.5 折
 * - 银卡会员 9 折
 * - 金卡会员 8.5 折
 * - 钻石会员 8 折
 *
 * @param level - 会员等级（MemberLevel enum）
 * @param amount - 原始金额（单位：分）
 * @returns 折后金额（单位：分）
 */
function calculateDiscount(level: MemberLevel, amount: number): number {
  // ...
}

何时用英文

When to Use English

  • 变量名、函数名、类名 — 始终用英文命名，遵循团队命名规范
  • Git commit message — 参考下方 commit 规范
  • 开源项目注释 — 面向国际社区的项目，注释统一用英文
  • 错误信息和日志 — 生产环境的 error message 用英文（避免编码问题）
  • API 接口文档 — 对外暴露的 API 用英文
  • Variable Names, Function Names, Class Names — Always use English naming, following team naming conventions
  • Git commit message — Refer to the commit guidelines below
  • Open Source Project Comments — For projects targeting the international community, use unified English comments
  • Error Messages and Logs — Use English for production environment error messages (to avoid encoding issues)
  • API Interface Documents — Use English for externally exposed APIs

混排格式要求

Mixed Format Requirements

typescript
// 好：中英文之间加空格
// 使用 Redis 缓存来减少 MySQL 的查询压力

// 坏：中英文之间没有空格
// 使用Redis缓存来减少MySQL的查询压力

// 好：技术术语保留英文
// 这里用 debounce 防抖处理，避免频繁触发 API 请求

// 坏：强行翻译技术术语
// 这里用防抖动处理，避免频繁触发应用程序接口请求
typescript
// Good: Add space between Chinese and English
// 使用 Redis 缓存来减少 MySQL 的查询压力

// Bad: No space between Chinese and English
// 使用Redis缓存来减少MySQL的查询压力

// Good: Keep technical terms in English
// 这里用 debounce 防抖处理，避免频繁触发 API 请求

// Bad: Forcibly translate technical terms
// 这里用防抖动处理，避免频繁触发应用程序接口请求

Commit Message 中英双语格式

Bilingual Chinese-English Commit Message Format

推荐格式

Recommended Format

团队内部项目使用中文 commit message，采用约定式提交（Conventional Commits）的中文版：
<类型>(<范围>): <简要描述>

<详细说明（可选）>

<关联信息（可选）>
For internal team projects, use Chinese commit messages, adopting the Chinese version of Conventional Commits:
<类型>(<范围>): <简要描述>

<详细说明（可选）>

<关联信息（可选）>

类型对照表

Type Comparison Table

类型含义示例
feat新功能feat(用户): 新增手机号登录功能
fix修复 Bugfix(支付): 修复微信支付回调重复处理的问题
docs文档变更docs: 更新 API 接口文档
style代码格式style: 统一缩进为 2 个空格
refactor重构refactor(订单): 拆分订单服务，提取公共逻辑
perf性能优化perf(列表): 虚拟滚动优化长列表渲染性能
test测试test(auth): 补充登录模块单元测试
chore构建/工具chore: 升级 Node.js 至 v20
TypeMeaningExample
featNew Featurefeat(用户): 新增手机号登录功能
fixBug Fixfix(支付): 修复微信支付回调重复处理的问题
docsDocument Changesdocs: 更新 API 接口文档
styleCode Formatstyle: 统一缩进为 2 个空格
refactorRefactoringrefactor(订单): 拆分订单服务，提取公共逻辑
perfPerformance Optimizationperf(列表): 虚拟滚动优化长列表渲染性能
testTestingtest(auth): 补充登录模块单元测试
choreBuild/Toolschore: 升级 Node.js 至 v20

示例

Example

fix(支付): 修复支付宝异步回调签名校验失败的问题

原因：升级 SDK 后签名算法从 RSA 变为 RSA2，但回调校验仍使用旧算法。
方案：回调处理中同时兼容 RSA 和 RSA2 签名校验。

Closes #1234
fix(支付): 修复支付宝异步回调签名校验失败的问题

原因：升级 SDK 后签名算法从 RSA 变为 RSA2，但回调校验仍使用旧算法。
方案：回调处理中同时兼容 RSA 和 RSA2 签名校验。

Closes #1234

面向国际社区的项目

Projects Targeting the International Community

如果项目面向国际社区或有外籍成员，commit message 用英文，PR 描述中可附加中文说明：
fix(payment): fix Alipay async callback signature verification failure

The SDK upgrade changed the signature algorithm from RSA to RSA2,
but the callback handler still used the old algorithm.

Closes #1234
If the project targets the international community or has foreign members, use English commit messages, and you can attach Chinese explanations in the PR description:
fix(payment): fix Alipay async callback signature verification failure

The SDK upgrade changed the signature algorithm from RSA to RSA2,
but the callback handler still used the old algorithm.

Closes #1234

常见反模式与对策

Common Anti-Patterns and Countermeasures

反模式一：过度客气

Anti-Pattern 1: Overly Polite

表现： 所有评论都是"我觉得可能也许大概好像这里有个小问题"。
后果： 关键 bug 被隐藏在一堆委婉语里，作者根本不知道哪些必须改。
对策： 使用分级标注。[必须修复] 就是必须修复，语气可以温和，但级别必须准确。
undefined
Performance: All comments are like "I think maybe perhaps there's a small problem here".
Consequence: Critical bugs are hidden in a pile of euphemisms, and the author doesn't know which ones must be fixed.
Countermeasure: Use priority labeling. [Must Fix] means it must be fixed; the tone can be gentle, but the level must be accurate.
undefined

坏：过度客气

Bad: Overly polite

不知道我理解得对不对，这里好像可能有一点点并发问题，不过也许我看错了...
I don't know if I understand correctly, but it seems like there might be a tiny concurrency issue here, though maybe I'm wrong...

好：温和但清晰

Good: Gentle but clear

[必须修复] 并发安全问题
这里的 map 在多个 goroutine 中同时读写，会触发 panic。 建议加 sync.RWMutex，或者换成 sync.Map。
复现方式：加 -race flag 跑测试就能看到。
undefined
[必须修复] 并发安全问题
这里的 map 在多个 goroutine 中同时读写，会触发 panic。 建议加 sync.RWMutex，或者换成 sync.Map。
复现方式：加 -race flag 跑测试就能看到。
undefined

反模式二：不敢给高级开发者提意见

Anti-Pattern 2: Afraid to Give Feedback to Senior Developers

表现： 高级开发者或 Leader 的代码直接 Approve，不仔细看。
后果： 代码质量双标，团队对 Code Review 失去信任。
对策： Code Review 对事不对人。可以换个表达方式：
undefined
Performance: Directly Approve code from senior developers or leaders without careful review.
Consequence: Double standards in code quality, and the team loses trust in Code Review.
Countermeasure: Code Review is about the code, not the person. You can use an alternative expression:
undefined

提问式（适合给资深同事的反馈）

Question-based (suitable for feedback to senior colleagues)

想请教一下，这里选择用递归而不是迭代，是出于什么考虑？ 我在想如果递归深度超过 1000 层会不会有栈溢出的风险？
I'd like to ask, what's the consideration for choosing recursion instead of iteration here? I'm wondering if stack overflow will occur if the recursion depth exceeds 1000 layers?

学习式

Learning-based

学到了一个新写法！不过有个小疑问——这里的类型断言在运行时不会做检查， 如果上游数据结构变了，这里会静默通过。是否考虑加个 runtime validation？
undefined
I learned a new writing style! But I have a small question—this type assertion isn't checked at runtime. If the upstream data structure changes, this will pass silently. Have you considered adding runtime validation?
undefined

反模式三：审查变成风格之争

Anti-Pattern 3: Review Becomes a Style Debate

表现： 大量评论纠结于缩进、空格、花括号位置。
后果： 浪费时间，忽略真正的问题。
对策： 风格问题交给 ESLint / Prettier / gofmt 等工具自动处理。Code Review 聚焦逻辑、安全、性能。
Performance: A large number of comments focus on indentation, spaces, and brace positions.
Consequence: Wastes time and ignores real issues.
Countermeasure: Leave style issues to tools like ESLint / Prettier / gofmt to handle automatically. Code Review should focus on logic, security, and performance.

反模式四：只写"LGTM"

Anti-Pattern 4: Only Write "LGTM"

表现： 随手一个 LGTM 就 Approve，没有实质性审查。
后果： Code Review 形同虚设，出了问题没人兜底。
对策： 即使代码质量很好，也要写出你关注了哪些方面：
LGTM

审查了以下方面：
- 并发安全：锁的粒度合理
- 错误处理：所有外部调用都有 error handling
- 向下兼容：新增字段都有默认值，不影响老版本

一个小建议 [仅供参考]：第 78 行的变量名 `d` 可以改成 `duration`，更易读。
Performance: Casually write LGTM and Approve without substantive review.
Consequence: Code Review becomes a formality, and no one takes responsibility when problems arise.
Countermeasure: Even if the code quality is good, write down which aspects you reviewed:
LGTM

Reviewed aspects:
- Concurrency safety: Lock granularity is reasonable
- Error handling: All external calls have error handling
- Backward compatibility: New fields have default values and do not affect old versions

A small suggestion [仅供参考]: The variable name `d` on line 78 can be changed to `duration` for better readability.

审查流程建议

Review Process Recommendations

开始审查前

Before Starting the Review

  1. 先看 PR 描述，理解改动的背景和目的
  2. 看关联的 Issue 或需求文档
  3. 先整体浏览，再逐文件细看
  1. First read the PR description to understand the background and purpose of the changes
  2. Read the associated Issue or requirement document
  3. First browse the overall content, then read each file in detail

审查顺序

Review Order

  1. 架构层面 — 方案是否合理？有没有更好的方式？
  2. 正确性 — 逻辑对不对？边界条件处理了吗？
  3. 安全性 — 有没有注入、越权、信息泄露？
  4. 性能 — 有没有 N+1 查询、内存泄漏、不必要的循环？
  5. 可维护性 — 半年后能看懂吗？测试覆盖了吗？
  6. 风格 — 只关注工具无法自动处理的部分
  1. Architecture Level — Is the solution reasonable? Is there a better way?
  2. Correctness — Is the logic correct? Are boundary conditions handled?
  3. Security — Are there injection, privilege escalation, or information leakage issues?
  4. Performance — Are there N+1 queries, memory leaks, or unnecessary loops?
  5. Maintainability — Can it be understood in half a year? Is test coverage sufficient?
  6. Style — Only focus on parts that cannot be handled automatically by tools

给出总结

Provide a Summary

审查结束后，给一段总结，包括：
  • 整体评价（一句话）
  • 值得学习的地方（先扬后抑）
  • 主要问题列表（按优先级）
  • 建议的修改方向
总结：整体实现思路清晰，支付回调的幂等处理很到位。

主要问题：
1. [必须修复] 并发写 map 的问题（2 处）
2. [建议修改] 缺少对空值的校验（3 处）
3. [仅供参考] 几个变量命名可以更语义化

建议先修复并发问题，校验的部分可以本次一起改或者拆到下个迭代。
After the review, provide a summary including:
  • Overall evaluation (one sentence)
  • Aspects worth learning (praise first, then critique)
  • List of main issues (by priority)
  • Suggested modification directions
Summary: The overall implementation logic is clear, and the idempotent processing of payment callbacks is well done.

Main Issues:
1. [Must Fix] Concurrent map write issues (2 places)
2. [Suggested Modification] Missing null value validations (3 places)
3. [For Reference Only] Several variable names can be more semantic

Suggestion: Fix the concurrency issues first; the validation part can be fixed in this iteration or split into the next iteration.

检查清单

Checklist

在提交审查意见前，确认：
  • 每条评论都标注了优先级
  • [必须修复] 的问题都给出了具体的修复建议
  • 没有因为面子而跳过关键问题
  • 没有纠结于工具能自动处理的风格问题
  • 对好的代码给予了肯定
  • 给出了整体总结
Before submitting review comments, confirm:
  • Each comment is labeled with priority
  • Specific repair suggestions are provided for [Must Fix] issues
  • No critical issues were skipped for the sake of saving face
  • No style issues that can be handled automatically by tools were debated
  • Good code was praised
  • An overall summary was provided ",