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, or copying the straightforward Western style which embarrasses colleagues. This skill 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 politeness.

审查反馈的表达方式

Expression of Review Feedback

用建议代替命令

Replace Commands with Suggestions

避免（命令式）推荐（建议式）
你必须改成 X建议考虑用 X，因为 Y
这里写错了这里可能存在一个问题，是否考虑过 Z 的情况？
不要用这个方法这个方法在 A 场景下可能有性能问题，可以看看 B 方案
这段代码不行这段逻辑我理解得对吗？如果输入为空的话会怎样？
Avoid (Imperative)Recommend (Suggestive)
You must change this to XIt is recommended to consider using X because Y
This is written 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 at solution B
This code doesn't workDid I understand this logic correctly? What happens if the input is empty?

用提问代替否定

Replace Negations with Questions

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

好的方式

Good approach

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

不好的方式

Bad approach

这里不应该用 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 Change] — Performance issues, maintainability, missing validations (fix in this iteration or next)
  • [For Reference Only] — Naming optimizations, style suggestions, alternative solutions (okay to leave unchanged)
  • [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 Guidelines

何时用中文

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 requirement sources
  • Complex Algorithm Comments — Write the thought process in Chinese to ensure all team members can understand
  • TODO / FIXME — Describe pending tasks in Chinese for easy searching and tracking
  • Document Comments (Internal Projects) — Use Chinese for descriptive text in JSDoc / Javadoc
typescript
/**
 * Calculate member level discount for users
 *
 * Business Rules:
 * - Regular members: 5% off
 * - Silver members: 10% off
 * - Gold members: 15% off
 * - Diamond members: 20% off
 *
 * @param level - Member level (MemberLevel enum)
 * @param amount - Original amount (unit: cents)
 * @returns Discounted amount (unit: cents)
 */
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, follow team naming conventions
  • Git commit message — Refer to the commit guidelines below
  • Open Source Project Comments — For projects targeting the international community, use English for comments uniformly
  • Error messages and logs — Use English for production environment error messages (to avoid encoding issues)
  • API Documentation — Use English for externally exposed APIs

混排格式要求

Mixed Format Requirements

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

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

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

// 坏：强行翻译技术术语
// 这里用防抖动处理，避免频繁触发应用程序接口请求
typescript
// Good: Add space between Chinese and English
// Use Redis cache to reduce MySQL query pressure

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

// Good: Keep technical terms in English
// Use debounce to prevent frequent API requests

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

Commit Message 中英双语格式

Commit Message Chinese-English Bilingual Format

推荐格式

Recommended Format

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

<详细说明（可选）>

<关联信息（可选）>
For internal team projects, use Chinese commit messages, adopting the Chinese version of Conventional Commits:
<type>(<scope>): <brief description>

<detailed explanation (optional)>

<relevant information (optional)>

类型对照表

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(user): Add mobile phone login function
fixFix bugfix(payment): Fix repeated processing of WeChat payment callbacks
docsDocument changesdocs: Update API documentation
styleCode formatstyle: Unify indentation to 2 spaces
refactorRefactoringrefactor(order): Split order service and extract common logic
perfPerformance optimizationperf(list): Virtual scroll optimization for long list rendering performance
testTestingtest(auth): Add unit tests for login module
choreBuild/toolschore: Upgrade Node.js to v20

示例

Example

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

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

Closes #1234
fix(payment): Fix Alipay asynchronous callback signature verification failure

Reason: After upgrading the SDK, the signature algorithm changed from RSA to RSA2, but callback verification still used the old algorithm.
Solution: Support both RSA and RSA2 signature verification in callback processing.

Closes #1234

面向国际社区的项目

Projects Targeting 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 for commit messages, and attach Chinese explanations in PR descriptions:
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 issue 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 priority must be accurate.
undefined

坏：过度客气

Bad: Overly polite

不知道我理解得对不对，这里好像可能有一点点并发问题，不过也许我看错了...
I don't know if I understand correctly, but 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
[Must Fix] Concurrency Safety Issue
This map is being read and written simultaneously in multiple goroutines, which will trigger a panic. It is recommended to add sync.RWMutex, or switch to sync.Map.
Reproduction method: Run tests with -race flag to see it.
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 was the consideration for choosing recursion instead of iteration here? I'm thinking if the recursion depth exceeds 1000 layers, will there be a stack overflow risk?

学习式

Learning-based

学到了一个新写法！不过有个小疑问——这里的类型断言在运行时不会做检查， 如果上游数据结构变了，这里会静默通过。是否考虑加个 runtime validation？
undefined
Learned a new writing style! But I have a small question — the type assertion here 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 Dispute

表现： 大量评论纠结于缩进、空格、花括号位置。
后果： 浪费时间，忽略真正的问题。
对策： 风格问题交给 ESLint / Prettier / gofmt 等工具自动处理。Code Review 聚焦逻辑、安全、性能。
Performance: A large number of comments obsess over indentation, spaces, brace positions.
Consequence: Wastes time and ignores real issues.
Countermeasure: Leave style issues to tools like ESLint / Prettier / gofmt to handle automatically. Code Review focuses 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 occur.
Countermeasure: Even if the code quality is good, write out the aspects you focused on:
LGTM

Reviewed the following aspects:
- Concurrency safety: Lock granularity is reasonable
- Error handling: All external calls have error handling
- Backward compatibility: New fields all have default values, no impact on old versions

A small suggestion [For Reference Only]: The variable name `d` on line 78 can be changed to `duration` for better readability.

审查流程建议

Review Process Suggestions

开始审查前

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. Look at related Issues or requirement documents
  3. First browse the overall changes, then look at 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, unauthorized access, information leakage issues?
  4. Performance — Are there N+1 queries, memory leaks, 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 completing the review, provide a summary including:
  • Overall evaluation (one sentence)
  • Points worth learning (praise first, then constructive feedback)
  • List of main issues (by priority)
  • Suggested modification directions
Summary: The overall implementation idea 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 Change] Missing null value validations (3 places)
3. [For Reference Only] Several variable names can be more semantic

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

检查清单

Checklist

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