这个拉取请求静静地躺在那里，包含了47个文件变更，描述写着”实现新的计费模块”。你打开它，滚动查看差异，然后开始做大多数工程师都会做的事情：检查拼写错误，验证变量名，并指出某个函数可能应该是私有的。二十分钟后，你批准了它。两周后，计费模块导致了服务中断，因为它没有处理支付网关集成中的部分故障，而在代码审查中没有人发现这个问题。
这是大多数代码审查流程的失败模式。团队将审查视为语法和风格检查——一种由人类执行的、经过美化的linter检查。但实际上，生产环境中真正重要的错误几乎从来不是来自命名约定或缺失的分号。它们来自架构错误、未处理的边缘情况、安全疏忽，以及仅在负载下才会显现的微妙性能问题。
资深工程师的代码审查方式不同。他们花较少的时间在表面问题上，而是更多时间提出问题，比如：”当这个外部服务宕机时会发生什么？”、”当表有5000万行时，这个查询还能保持性能吗？”以及”这个变更会让下一个功能更难构建吗？”
本指南介绍了经验丰富的工程师在代码审查时实际关注的内容，如何提供既能改进代码也能提升作者水平的反馈，以及如何构建不会拖慢团队速度的审查文化。
架构审查：这个功能应该放在这里吗？
在阅读任何代码行之前，先查看哪些文件被修改以及变更如何组织。这比任何单个函数更能告诉你一个PR的质量。
依赖方向：这个变更是否引入了方向错误的依赖？如果你的billing模块现在从notifications导入，这可能是一个设计问题。核心业务逻辑不应该依赖于基础设施或表示层。在结构良好的代码库中，依赖指向内部——从控制器到服务，从服务到领域模型，绝不应该相反。
责任边界：如果对用户档案功能的变更需要修改订单处理模块，那么这些模块之间的边界可能是错误的。一个涉及许多不相关模块的PR通常是紧密耦合的迹象，而毫无评论地批准它意味着接受这种耦合成为新的常态。
界面设计：当一个 PR 引入新功能、类或 API 端点时，应在实现前评估界面。函数签名是否清晰地表明了它接受和返回的内容？仅凭名称和参数，从未见过此代码的开发人员能否正确使用它？糟糕的界面会迅速固化——一旦其他代码依赖它们，更改签名就会变成一个更大的项目。
# 不佳：这个函数实际做什么？字典结构是什么？
def process(data: dict, flag: bool = False) -> dict:
...
# 更好：清晰的类型，清晰的目的
def calculate_order_total(
line_items: list[LineItem],
apply_discount: bool = False,
tax_region: TaxRegion = TaxRegion.US
) -> OrderTotal:
...“下一个功能”测试：思考团队下个季度需要构建什么。这个 PR 是让这项工作变得更容易还是更困难？一个仅满足当前需求的快速实现，却让团队在下一个功能上陷入困境，这不是一个好的权衡，即使代码本身很干净。
安全审查：大多数团队会跳过的检查
安全漏洞代价高昂。在生产环境中发现的 SQL 注入或身份验证绕过问题，其修复成本比在审查阶段发现的问题高出几个数量级。然而，大多数代码审查从不花时间在安全上，因为审查者假设”我们有安全工具来处理这些”。
自动化安全扫描器能捕获已知的漏洞模式。但它们无法捕获您身份验证流程中的逻辑错误或 API 中的授权漏洞。以下是手动需要检查的内容：
身份验证和授权：每个新的 API 端点都应有明确的授权检查。如果您看到没有验证用户权限中间件的新路由处理器，那是一个危险信号。不要假设框架已经处理了这一点——请验证。
// Express.js 示例 -- 找出问题
app.get('/api/admin/users', async (req, res) => {
// 授权检查在哪里？
// 任何已认证用户都可以访问管理员用户列表
const users = await UserService.getAllUsers();
res.json(users);
});
// 已修复
app.get('/api/admin/users', requireRole('admin'), async (req, res) => {
const users = await UserService.getAllUsers();
res.json(users);
});输入验证和清理：任何来自您信任边界之外的数据——用户输入、webhook 负载、来自合作伙伴 API 的数据——在使用前必须进行验证。检查 PR 是否验证了类型、范围和格式。注意用户输入未经清理就被插入到 SQL 查询、shell 命令或模板渲染中的情况。
机密信息和凭证：检查差异中是否有硬编码的 API 密钥、数据库密码或令牌。这似乎很明显，但这种情况发生的频率比任何人承认的都要高。还要检查可能被记录的机密信息——如果一个函数记录完整的请求对象，而该对象包含授权头，那么您的日志聚合器就会出现凭证泄露。
IDOR（不安全的直接对象引用）：当端点接受 ID 参数时，验证代码是否检查了请求用户是否有权访问该特定资源。一个常见错误：
// 易受攻击：任何经过身份验证的用户都可以查看任何发票
app.get('/api/invoices/:id', requireAuth, async (req, res) => {
const invoice = await Invoice.findById(req.params.id);
res.json(invoice);
});
// 已修复：验证所有权
app.get('/api/invoices/:id', requireAuth, async (req, res) => {
const invoice = await Invoice.findById(req.params.id);
if (!invoice || invoice.userId !== req.user.id) {
return res.status(404).json({ error: '未找到' });
}
res.json(invoice);
});性能审查：在负载下这将如何表现？
在开发环境中使用数据库中的 100 行数据运行良好的代码，在生产环境中面对 1000 万行数据时可能会彻底崩溃。在审查过程中，请考虑生产数据量和流量模式。
数据库查询：这是大多数性能问题所在。检查以下内容：
- N+1 查询：在每次迭代中执行数据库查询的循环。ORM 特别容易意外引入这个问题。如果您看到遍历集合的循环，其中每个项目都会触发延迟加载的关系，请标记它。
- 缺失索引：如果 PR 添加了一个新查询，其中 WHERE 子句作用于未索引的列，随着数据增长，它将变成全表扫描。检查迁移文件——是否有与新查询模式对应的索引？
- 无界查询：任何没有 LIMIT 子句的查询都是潜在的时间炸弹。
SELECT * FROM events WHERE user_id = ?看起来没问题，直到一个重度用户有 50 万个事件。
# Django ORM 中的 N+1 问题
# 这会执行 1 个查询获取订单 + N 个查询获取项目
orders = Order.objects.filter(status='pending')
for order in orders:
items = order.items.all() # 每次迭代都会访问数据库
process_order(order, items)
# 使用 select_related / prefetch_related 修复
orders = Order.objects.filter(status='pending').prefetch_related('items')
for order in orders:
items = order.items.all() # 没有额外查询 -- 已加载
process_order(order, items)内存使用：代码是否将整个数据集加载到内存中？注意类似没有分页的 Model.objects.all() 模式，当逐行处理可行时却将整个文件读入字符串，或者当生成器更合适时却构建大型列表等情况。
外部服务调用：如果 PR 在请求路径中添加了对外部 API 的调用，请考虑：超时时间是多少？如果服务响应缓慢或宕机会发生什么？是否有断路器？在结账流程中使用默认 30 秒超时的外部 API 调用，当该服务出现问题时，将会毁掉用户的一天。
并发性：如果代码读取一个值，基于它做出决策，然后写入——但其他线程或进程可能会在此期间修改该值——你就存在竞态条件。这在库存管理、账户余额更新以及任何”检查-然后-执行”模式中很常见。
# 竞态条件：两个并发请求都可能看到 quantity=1
# 并且都成功执行，导致产品超卖
def purchase_item(item_id, quantity):
item = db.query("SELECT quantity FROM inventory WHERE id = %s", item_id)
if item.quantity >= quantity:
db.execute("UPDATE inventory SET quantity = quantity - %s WHERE id = %s",
quantity, item_id)
return True
return False
# 使用原子操作修复
def purchase_item(item_id, quantity):
result = db.execute(
"UPDATE inventory SET quantity = quantity - %s "
"WHERE id = %s AND quantity >= %s",
quantity, item_id, quantity
)
return result.rowcount > 0错误处理：仅考虑正常情况是不够的
PR 中的大多数代码实现了正常路径——即一切工作正常时会发生什么。资深审查者会花费不成比例的时间来查看出现问题时的情况。
部分失败：如果一个函数执行三个步骤（向客户收费、创建订单、发送确认邮件），第二步失败了会发生什么？客户被收费但没有订单吗？是否有回滚机制？跨服务的分布式事务 notoriously 难以正确实现，审查它们需要思考每一种失败组合。
重试行为：如果代码重试失败的操作，该操作是幂等的吗？没有去重机制重试非幂等操作（如信用卡扣款）将造成实际问题。确保重试逻辑包含指数退避和最大重试次数。
错误消息：错误消息是否包含足够的调试上下文？凌晨 3 点显示”处理请求失败”的错误日志是无用的。而显示”无法向客户 cus_abc123 收费：Stripe API 返回 402，消息为’card_declined'”的错误日志是可以采取行动的。
优雅降级：当非关键依赖失败时，应用程序是优雅降级还是完全崩溃？如果推荐引擎宕机，产品页面仍然应该加载——只是没有推荐内容。检查代码以确保可选功能有适当的回退行为。
按 PR 类型的审查清单
并非所有的拉取请求都需要相同级别的审查。README 中的拼写错误修复不需要安全审查。新的支付集成需要全面审查。以下是按 PR 类型组织的实用清单：
功能 PR
- 功能是否符合规范或工单要求？
- 是否有足够的测试覆盖正常路径和边缘情况？
- 数据库迁移是否可逆？在大型数据集上是否会锁定表？
- 新的 API 端点是否有文档（OpenAPI 规范、README 等）？
- 是否支持功能标志以实现逐步发布？
- 对于拥有现有数据的老用户，此功能的行为如何？
- 是否为新功能添加了监控/警报？
错误修复 PR
- 在修复之前是否有重现该错误的测试？
- 修复是否解决了根本原因而不仅仅是症状？
- 代码库中其他地方是否存在同类错误？
- 修复是否向后兼容？是否会破坏现有客户端或集成？
- 是否存在回归风险？现有测试是否仍然通过？
重构 PR
- 重构是否与行为变化隔离？（切勿在同一 PR 中混合重构和功能更改。）
- 现有测试是否无需修改就能通过？如果测试需要更改，为什么？
- 重构是否真正改善了某些可衡量的方面——可读性、性能、可测试性——还是只是有所不同？
- 重构是否完成，还是让代码库处于不一致状态，其中一些代码使用旧模式，一些使用新模式？
基础设施/DevOps PR
- 基础设施变更是否有回滚程序文档？
- 是否已适当设置资源限制（CPU、内存、磁盘）？
- 机密信息是否通过机密管理器管理，而不是硬编码或检查到版本控制的环境文件中？
- 在生产环境之前，是否有办法在暂存环境中测试此变更？
- 如果此变更失败，影响范围有多大？是影响所有服务还是仅一个服务？
如何提供具有教学意义的反馈
撰写审查评论的方式与内容本身同样重要。代码审查是同事之间的对话，而不是守门人的判决。糟糕的审查文化是破坏团队士气和减缓交付速度的最快方式之一。
区分阻塞式和非阻塞式评论。 使用清晰的标记或标签系统。在许多团队中，这看起来像：
blocker:必须在合并前修复。安全问题、数据丢失风险、正确性错误。suggestion:我认为这样会更好，但这取决于你。风格偏好、替代方法。nit:微小问题。拼写错误、格式、命名偏好。不要仅因 nit 而阻塞 PR。question:我不理解这一点。帮助我学习，或者为下一个人添加评论。
没有这种区分，每条评论都感觉像是阻塞项，作者花费时间处理琐碎反馈，而真正的问题则在噪音中被忽略。
解释”为什么”，而不仅仅是”是什么”。 与其说”在这里使用常量”，不如说”这个超时值出现在三个地方。如果以后需要更改它，我们必须找到并更新所有这些地方。命名常量会使这更容易，并记录意图。”第一个版本告诉作者做什么。第二个版本教给他们可以应用于未来代码的原则。
提问而不是提出要求。 “如果这个列表为空会发生什么？”比”你需要处理空列表情况”更好。问题邀请作者思考问题。有时他们会意识到有一个错误。有时他们会解释为什么列表永远不会为空，你们都会学到一些东西。
认可良好的工作。 如果一段代码结构良好、 thoughtful 地处理边缘情况或使用巧妙的技术，请说出来。100% 是批评的评论会创造一种防御性文化，让人们害怕打开 PR。一句快速的”这里的方法很好——带指数退避的重试逻辑完全正确”不需要成本，并且强化了良好的实践。
不要在评论中重写他们的代码。 如果你发现自己正在写一个20行的代码片段作为评论，说明出了问题。要么这个问题足够重要值得进行对话（走到他们桌旁，或者打个电话），要么你应该打开自己的 PR 提出建议的更改。评论中的长代码块难以阅读、难以讨论，而且常常显得居高临下。
审查速度：快速前进但不走捷径
缓慢的代码审查会扼杀生产力。如果一个 PR 两天没有被审查，作者已经切换到其他事情上了。当审查评论最终到达时，他们必须重新加载原始更改的心理上下文，处理反馈，然后再次等待。这种来回可以将一个一天的功能变成一周的功能。
以下是保持审查速度的具体实践：
设定响应时间 SLA。 许多表现优异的团队目标是在 4 个工作小时内完成首次审查。这并不意味着审查在 4 小时内完成——而是指作者能够获得初步反馈，并知道接下来要做什么。谷歌的工程指南建议，审查者最迟应在一个工作日内提供反馈。
保持 PR 小型化。 你能为审查质量和速度做的最有效的事情就是将有意义的变更控制在 400 行代码以内。微软的研究（发表在他们的”现代代码审查”论文中）发现，在约 200-400 行的差异之后，审查效果会显著下降。超过这个范围，审查者开始浏览而不是仔细阅读。如果一个功能需要 2000 行的变更，将其分解为一系列相互构建的小型 PR。
使用草稿 PR 获取早期反馈。 如果你对某种架构方法不确定，可以先打开一个包含框架的草稿 PR，在编写所有测试和错误处理之前请求反馈。早期就方法达成一致可以避免完全实现的 PR 因基本设计原因而被拒绝的痛苦场景。
自动化一切可自动化的内容。 代码检查、格式化、类型检查和基本测试执行应该在 CI 中进行，而不是在审查中。人类每花一分钟指出格式问题，就是少花一分钟在架构或逻辑审查上。到 2026 年，没有自动化格式化的借口是不可接受的。对 JavaScript/TypeScript 使用 Prettier，对 Python 使用 Black 或 Ruff，对 Go 使用 gofmt，并在 CI 中强制执行。完全从审查中移除风格讨论。
# .github/workflows/ci.yml -- 自动化无聊的工作
name: CI
on: [pull_request]
jobs:
checks:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Lint
run: npx eslint . --max-warnings 0
- name: Format check
run: npx prettier --check .
- name: Type check
run: npx tsc --noEmit
- name: Test
run: npm test -- --coverage --ci轮换审查者。 不要让所有审查都通过一位高级工程师。这会造成瓶颈，使该人员倦怠，并阻止团队其他成员发展审查技能。轮流分配审查者或按所有权区域分配，并且只在高风险变更（安全敏感代码、数据库迁移、公共 API 变更）中使用必需的审查者。
构建健康的审查文化
代码审查文化最终关乎信任。作者是否信任审查者是在尝试帮助，而不是设卡？审查者是否信任作者已经考虑过他们的变更？建立这种信任需要有意为之的努力。
先审查自己的代码。 在请求审查之前，像审查者一样通读自己的代码差异。你会发现明显的问题、遗留的调试语句和本应解决的 TODO 注释。这体现了对审查者时间的尊重，并为需要检查的内容设定了更高的基准。
编写好的 PR 描述。 一个只写着”修复 bug”的单行描述 PR 是在要求审查者从代码差异中逆向推导意图。一个好的描述应包括：变更内容、为何需要（链接到工单或问题）、高级工作原理、你考虑过但决定不采用的方法，以及如何测试。这些上下文能显著加快审查速度并带来更好的反馈。
不要将审查评论个人化。 这条建议说起来容易做起来难，尤其是对经验较少的开发者而言。记住评论是针对代码而非你本人，这很有帮助。当审查者遵循上述反馈实践时也很有帮助——将评论表述为问题和建议，而非批评。
将审查视为学习机会。 每个 PR 都是学习新知识的机会——新库、解决问题的不同方法、你不熟悉的领域概念。最好的工程团队将审查视为知识共享机制，而不仅仅是质量关卡。初级工程师通过观察资深工程师如何构建代码来学习。资深工程师则通过初级工程师带来的新视角和新工具来学习。
衡量并改进。 跟踪首次审查时间、审查周期时间（从首次审查到合并）和 PR 大小分布等指标。这些数字告诉你审查流程是否健康或正在成为瓶颈。如果平均合并时间在六个月内从 1 天增加到 5 天，这就是一个调查信号。可能是 PR 变得太大，可能是团队需要更多审查者，也可能是审查标准变得过高。
代码审查不是什么
代码审查不是测试的替代品。如果你发现自己依赖审查者来捕获测试本应发现的错误，那么你的测试覆盖率就不足。审查是用于测试难以验证的内容：设计质量、可维护性、安全态势以及与团队架构方向的一致性。
代码审查不是设计审查。当代码编写完成时，设计决策基本已经确定。如果你不同意基本方法，提出异议的时机应该在设计阶段——在 RFC、设计文档或早期草案 PR 中——而不是在作者已经花了一周时间实现之后。
代码审查不是状态报告。如果你的经理通过阅读 PR 来了解团队的工作内容，这是一个流程问题，而非代码审查问题。
最后思考
一支代码审查做得好与做得差的团队之间的区别不在于工具（GitHub、GitLab、Gerrit——它们都很好用），而在于思维方式。优秀的审查团队明白，目标是集体代码所有权和持续改进，而非个人审批或把关。
如果你是高级工程师，你的审查评论会影响初级工程师对代码的思考方式。认真对待这份责任。不要只指出问题——要解释原因，分享教会你关注特定问题的经验，并明确提问不仅是可接受的，而且是应该的。
如果你职业生涯尚浅，应积极寻求审查反馈，研究有经验的审查者如何评论他人的 PR。你学到的模式——检查竞态条件、思考故障模式、评估接口设计——会比几乎所有其他实践更快地让你成为一名更好的工程师。
正确进行的代码审查是软件工程中价值最高的活动之一。它能在问题到达生产环境前捕获它们，在团队中传播知识，随着代码库增长保持架构一致性，并创造共同的质量标准，使每个人的工作都变得更好。在这上面投入吧。