代码评审清单要能直接用于判断,而不只是罗列注意事项。一份可用的清单通常包含五类信息:评审对象与范围、提交前准备、评审中的检查项、结论与合入条件、证据与负责人记录。它适用于单次代码变更的评审与合入,由作者、评审人和合入执行人分时点使用;代码优化方案和算法设计不在清单覆盖范围内。
要点总览(TL;DR)
- 一份可用的代码评审清单包含五类信息:评审对象与范围、提交前准备、评审中的检查项、结论与合入条件、证据与负责人记录。
- 清单按三个时点使用:作者提交评审前完成自查,评审人评审中逐项核对,合入执行人在合入前确认审批、流水线与回退方案。
- 评审中的检查项覆盖正确性、接口与兼容性、安全与权限、性能与资源、可读性与可维护性、测试覆盖,每项都要能判断通过与否并留下证据。
- 结论分为通过、有条件通过、退回修改三档;构建或测试失败、高风险问题未处理、关键审批缺失、回退方案未确认属于阻断项,必须清零后才能合入。
- 清单完成的判定条件是所有必检项有明确结论、遗留问题有责任人和处理时点、阻断项全部解除、合入条件逐项确认并记录执行人。
谁在什么时点使用这份评审清单
评审清单按三个时点使用:作者提交评审前、评审人评审中、合入执行前。每个时点对应不同的检查责任,混用会让清单失去约束力。作者在提交前完成自查并补齐变更信息;评审人依据检查项逐条核对并给出结论;合入执行人确认审批、流水线和遗留问题状态后再执行合入。
| 使用时点 | 主要使用者 | 检查责任 | 产出 |
|---|---|---|---|
| 提交评审前 | 作者 | 变更说明、影响范围、自检结果、风险提示是否完整 | 可进入评审的变更单 |
| 评审中 | 评审人 | 逐项核对正确性、兼容性、安全、性能、可维护性与测试覆盖 | 评审意见与结论 |
| 合入前 | 合入执行人 | 审批是否完成、流水线是否通过、冲突是否解决、回退方案是否确认 | 合入记录 |
清单的覆盖范围限定为单次变更的评审与合入。它帮助团队对一次变更形成一致判断,代码优化建议、算法选型和架构重构方案应放到其他评审或设计文档中讨论,避免清单条目膨胀到无法逐项核验。
提交评审前:作者需要准备并自查的信息
评审开始前,作者需要让评审人无需追问就能理解这次变更。准备项写成可核验的条目,每项标注通过标准、证据和负责人,信息不完整时评审人可以要求补充后再开始。
| 检查项 | 通过标准 | 证据 | 负责人 |
|---|---|---|---|
| 变更说明 | 写明改了什么、为什么改,并关联需求或缺陷编号 | 变更单描述、需求或缺陷链接 | 作者 |
| 变更范围 | 列出影响模块、接口、配置项、数据结构和依赖变化 | 变更文件清单、影响面说明 | 作者 |
| 本地构建 | 构建通过,无新增编译错误或警告 | 构建日志 | 作者 |
| 单元测试 | 相关测试已执行并通过,失败项有说明 | 测试结果记录 | 作者 |
| 静态检查 | 静态检查已执行,新增问题已处理或标注原因 | 检查报告 | 作者 |
| 风险提示 | 标出需要重点看的文件、已知遗留问题和待确认点 | 变更单中的风险说明 | 作者 |
风险提示是常被省略的一项。作者最清楚哪些改动容易出错,把重点文件和待确认点写清楚,评审人可以把时间放在高风险区域,减少在格式和命名上的反复评论。
评审中:逐项检查内容与通过标准
评审中的检查项是清单主体。每项都要能回答“通过还是不通过”,并留下证据。以下检查表可按变更类型增减条目,但增减需要在团队内说明理由。
| 检查维度 | 检查内容 | 通过标准 | 证据 | 检查人 |
|---|---|---|---|---|
| 正确性 | 逻辑与需求一致,边界条件和异常分支已处理 | 主要路径和异常路径均有对应处理,无遗漏分支 | 代码差异、评审评论 | 评审人 |
| 接口与兼容性 | 对外接口、数据结构、配置项变更是否向后兼容 | 不兼容变更已标注影响范围并给出迁移或版本策略 | 接口说明、变更记录 | 评审人 |
| 安全与权限 | 输入校验、敏感信息处理、权限判断、日志脱敏 | 外部输入有校验,敏感信息未明文暴露,权限判断在服务端完成 | 代码差异、安全检查记录 | 评审人或安全把关人 |
| 性能与资源 | 循环、查询、缓存和并发场景是否有明显风险 | 高频路径无明显全表扫描、重复查询或无界缓存 | 代码差异、必要的压测或查询计划 | 评审人 |
| 可读性与可维护性 | 命名、注释、重复代码、模块职责 | 命名表达意图,关键逻辑有注释,重复逻辑已抽取或说明保留原因 | 评审评论 | 评审人 |
| 测试覆盖 | 新增或修改逻辑是否有对应测试,测试可运行 | 核心分支有测试覆盖,测试在本地或流水线中通过 | 测试代码、执行结果 | 评审人 |
判断通过时,评审人应把结论写在对应条目旁,而不是只在整体评论中写“看起来可以”。对于无法当场确认的条目,标记为待确认并指定回复人,避免在合入前遗漏。
评审结论与合入条件怎么记录
评审结束后,结论需要归入明确类别,合入条件逐项确认。结论分类建议使用三档:通过,表示所有必检项已满足;有条件通过,表示存在非阻断问题,需在约定时点前处理;退回修改,表示存在阻断问题,修改后重新评审。
| 记录字段 | 填写要求 | 示例 |
|---|---|---|
| 结论类别 | 通过、有条件通过或退回修改,三选一 | 有条件通过 |
| 遗留问题描述 | 具体到文件、函数或行为,避免笼统表述 | 某接口缺少超时处理 |
| 责任人 | 指定处理人,不写“相关同学” | 作者或指定跟进人 |
| 处理时点 | 写明日期或合入前、下个迭代等明确节点 | 合入前完成 |
| 是否阻断合入 | 明确标注阻断或非阻断 | 阻断 |
合入条件至少包括:所需审批已完成、流水线构建与测试通过、代码冲突已解决、回退方案已确认。合入时点和执行人也要记录,便于出现问题时定位变更来源。有条件通过的变更,合入执行人需要确认非阻断问题的处理时点已被接受,再执行合入。
证据留存与负责人分工
清单能否追溯,取决于每类检查项是否留下可查证据。证据形式不必复杂,但要能对应到具体条目和检查时点。
| 角色 | 负责的检查项 | 证据形式 | 记录位置 |
|---|---|---|---|
| 作者 | 变更说明、范围、自检结果、风险提示 | 变更单描述、构建与测试日志、静态检查报告 | 变更单或评审记录 |
| 评审人 | 正确性、兼容性、可读性、测试覆盖 | 评审评论、逐项结论 | 评审记录 |
| 安全或测试把关人 | 安全与权限、测试有效性 | 安全检查记录、测试报告 | 评审记录或测试系统 |
| 合入执行人 | 审批、流水线、冲突、回退方案 | 流水线结果、合入记录 | 合入记录 |
每条记录应标注检查时点,例如提交时间、评审完成时间、合入时间。保留要求由团队按变更风险设定,高风险变更保留完整评审评论和测试报告,常规变更至少保留结论、审批和流水线结果。评审记录、变更单和流水线结果之间应能相互关联,事后追溯时可以从合入记录回到评审意见。
在评审与合入流程需要统一承载时,可以把变更单、评审状态和合入条件放在同一工作项中管理,让作者提交、评审人记录结论、合入执行人核对条件的动作对应到同一记录。ONES 可用于承载这类工作项与评审记录的管理动作,具体字段和流程按团队现有配置设置。
阻断项与清单完成条件
阻断项是合入前必须清零的条件,出现以下任一情形应停止合入:构建或测试失败;存在未处理的高风险问题,例如安全漏洞、数据兼容性破坏;关键审批缺失;回退方案未确认。阻断项不因评审进度紧张而豁免,解除阻断需要有明确处理结果。
非阻断项可以随变更合入,但需要记录问题描述、责任人和处理时点,并纳入后续跟踪。非阻断项长期不处理会累积成风险,团队可在固定节点回顾其关闭情况。
清单判定完成的条件是:所有必检项都有明确结论,没有停留在“待确认”;遗留问题均有责任人和处理时点;阻断项已全部解除;合入条件逐项确认并记录执行人。清单本身也需要维护,当出现新的阻断情形、检查项频繁被跳过或团队技术栈发生变化时,应更新条目和通过标准,并在团队内同步变更原因。
常见问题
小团队变更量不大,也需要按这份清单逐项检查吗?
可以按变更风险调整检查深度,但不宜整份跳过。低风险变更(如文案修改、配置微调)可以简化提交前准备中的部分证据要求,评审中仍保留正确性、兼容性和测试覆盖三项核对;涉及接口、权限、数据结构的变更建议完整执行清单。调整方式应在团队内写明哪些条目可简化、由谁判断风险等级,避免每次评审临时决定。
评审人提出的是改进建议而非阻断问题,应该怎么记录?
把这类意见记为非阻断项,填写问题描述、责任人和处理时点,结论可归为有条件通过。合入执行人需要确认这些问题的处理时点已被作者接受,再执行合入。非阻断项应纳入后续跟踪,在固定节点回顾关闭情况;如果同一类建议反复出现,可以考虑把它升级为检查项或写入团队编码约定。
紧急修复需要快速合入,阻断项可以临时豁免吗?
阻断项对应的是合入后可能直接造成故障或安全风险的情形,例如构建失败、高风险漏洞未处理、回退方案未确认,这类条件不建议因进度紧张而跳过。紧急场景可以压缩的是评审范围和等待时间,例如缩小变更范围、指定固定评审人快速响应、提前准备回退方案。若确需先合入再补处理,应明确记录豁免人、豁免理由和补处理时点,并在事后回顾中检查处理结果。
清单条目应该由谁维护,多久更新一次?
清单通常由团队的技术负责人或评审规则维护人牵头更新,作者和评审人提供实际使用中的问题反馈。触发更新的情形包括:出现新的阻断情形、某些检查项频繁被跳过或流于形式、技术栈或发布方式发生变化。更新时同步修改检查项和通过标准,并向团队说明变更原因,避免清单版本与实际做法脱节。
评审记录和合入记录需要保存多久?
保留要求由团队按变更风险设定。高风险变更建议保留完整评审评论、测试报告和审批记录,便于事后追溯问题来源;常规变更至少保留结论、审批和流水线结果。记录之间应能相互关联,从合入记录可以回到对应的评审意见和变更单。具体保留期限可结合团队所在行业的合规要求确定。
