Penpot 仓库代码评审规范：五轴评审方法、严重度分级与合并前质量门禁实践

Penpot 仓库代码评审规范：五轴评审方法、严重度分级与合并前质量门禁实践

【免费下载链接】penpotPenpot: The open-source design platform for Product teams that need scalable collaboration.项目地址: https://gitcode.com/GitHub_Trending/pe/penpot

导读

本文基于 Penpot 仓库.opencode技能体系中用于合并前评审的code-review技能文档，系统拆解一套面向「人类 + AI Agent」协作场景的多维度代码评审方法论：它规定了正确性、可读性、架构、安全、性能五个评审轴，定义了从 Critical 到 Suggestion 的严重度分级与结构化的评审报告输出格式，并配套变更规模控制、依赖审查、常见合理化借口拆穿、红旗清单与复核清单等质量门禁。文章结合仓库内的 review 命令入口、security-and-hardening 安全技能、plan-review 计划评审技能、testing 测试技能 以及 CONTRIBUTING.md、AGENTS.md 中的协作约束与提交规范做交叉印证。读完本文，你将掌握一套可直接落地的评审纪律、一份可以复用的评审报告模板，以及判断「何时放行、何时拦截」的明确标准。

这套评审体系在仓库中扮演什么角色

Penpot 是采用 Clojure（后端）、ClojureScript（前端与导出器）、Rust（render-wasm 渲染）与 TypeScript（MCP、media-processor 等）构建的开源设计平台。本仓库的.opencode目录为 AI 编码代理（opencode）预置了一套完整的工程协作技能，其中 code-review/SKILL.md 的 frontmatter 声明其触发条件：任何变更进入 main 分支之前都应当评审，评审对象既可以是自己写的代码，也可以是其他 Agent 或人类产出的代码。

这份技能并非孤立存在，它与仓库内其他规范形成闭环：

• .opencode/commands/review.md 是评审命令的统一入口，它先判断评审对象：是计划（实现计划 / 设计文档 / 任务拆解）则加载plan-review技能，是代码（diff / PR / 变更）则加载code-review技能；同时要求先读 AGENTS.md 再开始评审，并跳过生成文件、纯 lockfile 变更等无关内容。
• CONTRIBUTING.md 规定 PR至少需要一次批准才能合并、默认 squash-merge（PR 标题即最终提交信息）、评审期间用新增提交回应意见而非 force-push。
• AGENTS.md 则从代理行为层约束：不 push、不修改远程、提交前必须先读对应 workflow memory。

也就是说，code-review技能是这套「计划评审 → 实现 → 代码评审 → 合并」流水线里最后一扇质量门。

评审的审批标准与核心理念

技能文档给出的审批标准非常明确，值得单独强调：

批准标准：只要一个变更确实提升了整体代码健康度就应批准，即便它并不完美。完美的代码不存在——目标是持续改进。不要因为「换成我会写成别的样子」而拦截变更。只要它改善了代码库并遵循项目约定，就批准它。

这与"必须零缺陷才能合并"的教条形成对比，评审的门槛是净改善而非绝对完美。围绕这一标准，文档提炼了四条贯穿所有评审轴的原则：

• DRY（不要重复自己）：每份知识只能有一个权威表示。同一逻辑出现在两处应提取为共享 helper、模型或类型。评审者要把重复逻辑标为必须修改项——它不是"只是相似"，而是将来必然分叉的隐患。
• KISS（保持简单）：能工作的最简单方案就是最好的方案，复杂度必须自己挣得位置。如果你需要超过一句话才能解释某段代码在做什么，它可能就过于复杂了，应在合并前推动简化。
• YAGNI（你不会需要它）：不为假设中的未来用例预先添加抽象、钩子或泛化。在第三次出现时才做泛化，而不是第一次。评审者应删除投机性泛化。
• 不要凭空发明问题：不要为了凑反馈而制造问题。每一条发现都必须是真实风险、真实的阅读障碍或真实的架构隐患，而不是假想问题或伪装成问题的个人风格偏好。

五轴评审：逐条展开检查清单

文档要求每次评审都从以下五个维度评估变更，本节逐轴展开并给出原文清单对应的检查点。

轴一：正确性（Correctness）

代码是否做到了它所声称的事情：

• 是否符合规格或任务要求？
• 边界情况是否处理（null、空值、边界值）？
• 错误路径是否处理（不能只覆盖 happy path）？
• 是否通过全部测试？测试是否真的在测正确的东西？
• 是否存在差一错误（off-by-one）、竞态条件或状态不一致？

轴二：可读性与简洁性（Readability & Simplicity）

另一位工程师（或 Agent）能否不靠作者讲解就理解这段代码：

• 命名是否具有描述性并与项目约定一致？（无上下文的temp、data、result是红旗）
• 控制流是否直白（避免嵌套三元、深层回调）？
• 是否存在应简化的"炫技"写法？
• KISS 检查：这是解决问题的最简方案吗？一段 20 行的直白函数优于一段 5 行但需要注释解释的"巧妙"函数。
• 能否用更少行数完成？（本可用 100 行却写了 1000 行是失败）
• 抽象是否挣到了它的复杂度？（第三次出现前不做泛化）
• 是否把新的条件分支硬接在无关流程上？应把逻辑推入独立的 helper、state 或 policy。
• 是否反复出现对同一形状数据的重复条件判断？这往往意味着缺少一个数据模型或 dispatcher。
• 是否存在死代码痕迹：无操作变量、向后兼容 shim、// removed之类注释。

轴三：架构（Architecture）

变更是否契合系统设计：

• 是沿用现有模式还是引入新模式？若引入新模式，是否站得住脚？
• 是否维护了清晰的模块边界？
• DRY 检查：是否已有做同样事情的既有代码？应复用权威 helper，而不是写一个近似重复品；若两个分支做几乎相同的事，应合并。
• 依赖方向是否正确（无循环依赖）？
• 抽象层级是否恰当（既不过度设计，也不过度耦合）？
• 这次重构是降低复杂度还是仅仅搬移复杂度？数一数读者需要同时记住的概念数量——优先选择能让整条分支消失的重构，而不是把同样的逻辑重新集中到一处；优先删除一个抽象，而不是打磨它。
• 特性专属逻辑是否泄漏进共享/通用模块？
• 类型边界是否明确？对无理由的any/unknown/可选项/强制转换/静默回退要提出质疑。
• 结构性补救：指出问题时给出迁移方案而不只是问题本身——用 dispatcher 替换条件链、合并重复分支、把编排逻辑与业务逻辑分离、提取 helper、拆分大文件。优先选择能移除"活动部件"的方案，而不是把同样复杂度摊得更开的方案。

轴四：安全（Security）

文档指出详细安全指引见security-and-hardening技能（仓库内对应 .opencode/skills/security-and-hardening/SKILL.md），评审轴聚焦于：

• 用户输入是否被校验与清洗？
• 机密信息是否被排除在代码、日志与版本控制之外？
• 需要的地方是否做了认证/授权检查？
• SQL 查询是否参数化（不做字符串拼接）？
• 输出是否编码以防止 XSS？
• 依赖是否来自可信来源且无已知漏洞？
• 外部来源数据（API、日志、用户内容、配置文件）是否一律视为不可信？

作为延伸，security-and-hardening技能在仓库内进一步提供了「先威胁建模（信任边界 → 资产 → STRIDE 六类威胁 → 把滥用用例写在正常用例旁边）」的流程、Always/Ask First/Never 三层边界、以及针对 SQL 注入、失效认证、XSS、越权访问、SSRF（含 DNS rebinding/TOCTOU 的坦诚说明）、npm audit 分流决策树与 LLM 应用安全（把模型输出视为不可信输入、提示注入、过度代理等）的完整加固方案——评审者在处理涉及用户输入、认证、存储或外部集成的变更时应联动阅读。

轴五：性能（Performance）

• 是否存在 N+1 查询模式？
• 是否存在无界循环或不受约束的数据拉取？
• 是否有本应异步却在同步执行的操作？
• UI 组件是否存在无谓的重复渲染？
• 列表端点是否缺少分页？
• 热路径上是否创建了大对象？

评审流程：从理解意图到归类发现

技能文档定义了五步评审流程：

1. 理解意图（Understand the intent）——这个变更要达成什么？实现的是哪份规格或任务？
2. 先看测试（Review tests first）——测试揭示意图与覆盖率。它们测的是行为还是实现细节？边界情况覆盖了吗？
3. 评审实现（Review the implementation）——带着五个轴逐文件走查。
4. 给发现分类（Categorize findings）——每条评论都打上严重度标签。
5. 验证"验证本身"（Verify the verification）——跑了哪些测试？构建通过了吗？是否手工验证过？UI 变更是否有截图？

第四步的严重度分级表格是整个评审能顺畅推进的关键基础设施：

对每一条发现，文档都要求描述触发失败的具体场景：是何种输入、何种负载条件、何种时序或用户操作会触发问题。判断标准是——"当输入为 null 时会崩溃"是可执行的发现；"它可能会崩"不是。这直接呼应了四条核心原则中的"不发明问题"。

优先级排序也有明确纪律：把最有价值的放在最前——正确性与安全问题优先，其次是结构性问题，最后才是其他；少数高置信度评论优于一长串灌水清单。这一点与 review 命令 中的强规则一致："一个结构性问题胜过十条吹毛求疵（nits）"，且"缺失测试是问题而不是建议——必须作为带严重度标签的发现上报"。

结构化评审输出：七段式报告模板

文档规定每次评审都应按以下格式输出，使评审结果对作者可执行、对管理者可仲裁。这也是本技能最直接可复用的部分：

Summary（摘要）

简述代码做什么，并给出整体评估。

Critical and High-Priority Issues（关键与高优先级问题）

列出可能导致安全事故、数据丢失、崩溃、错误行为或重大性能退化的问题。每条需包含：严重度、文件/函数/代码位置、为什么是问题、失败的触发场景，并在有用时给出带修正代码的具体改进建议。

Other Findings（其他发现）

列出中低优先级问题，包括可维护性与设计问题。

Suggested Refactoring（建议的重构）

给出聚焦的代码改动或修订片段；除非明确论证，否则保留既有行为。

Testing Recommendations（测试建议）

指出缺失的测试，并描述具体测试用例，包括边界情况与失败场景。这与仓库 testing 技能 的 TDD 纪律（RED → GREEN → REFACTOR）、bug 修复必须带复现测试的 Prove-It 模式、以及"测试是规格说明、DAMP 优于 DRY"等约定相互呼应。

Positive Observations（正面观察）

指出哪些实现选择是清晰、安全、高效或设计良好的。文档特别强调：这不是恭维话术——它强化好模式，并告诉作者该继续做什么。

Final Verdict（最终裁定）

三选一：

• Approve（批准）——可以合并
• Approve with minor changes（附带小改批准）——处理完低/中优先级问题后即可合并
• Request changes（要求修改）——Critical 或 High 级问题必须在合并前解决

变更规模控制：评审友好度与拆分策略

文档明确主张"小而聚焦的变更容易评审、更快合并、更安全部署"，并给出量级参考：

~100 行改动 → 良好。一次坐定即可评审完。 ~300 行改动 → 可接受，前提是单个逻辑变更。 ~1000 行改动 → 过大。拆开。

同时提醒要盯文件总行数而不只是 diff 大小：单个文件累计约 1000 行通常就是一个需要警惕的信号；当一次改动实质性地撑大本已庞大的文件时，应先做分解。文档给出四种拆分策略：

一条铁律：把重构与功能开发分开。一个既做重构又加新行为的变更实际上是两个变更——应分别提交。这种"一次一个逻辑变更"的粒度要求，也呼应了 CONTRIBUTING.md 中"不要在一个 PR 里混杂无关变更"的协作规范。

变更描述（Change Descriptions）规范

• 首行：短小、祈使句、自包含。写 "Delete the FizzBuzz RPC"，而不是 "Deleting the FizzBuzz RPC."
• 正文：说明改了什么、为什么改，包含代码本身无法体现的上下文与推理。
• 反模式："Fix bug"、"Fix build"、"Add patch"、"Phase 1" 这类模糊描述。

这一规范与仓库提交约定同源：仓库根目录 CONTRIBUTING.md 规定提交信息为:emoji: <subject>（祈使语气、首字母大写、不加句号），并列出 18 种提交 emoji 类型表；仓库提供了机器校验工具 scripts/check-commit（Python 实现），其检查器包括：匹配 CI 正则（允许:emoji: <大写主语且无结尾句点>或Merge|Revert|Reapply前缀）、主语不超过 90 字符、主语无尾句点、主语大写、主语与正文间空行、以及代码变更必须包含Signed-off-by:（DCO）。注意一个细节差异：CONTRIBUTING 建议主语不超过 70 字符，而 check-commit 脚本的硬性上限是 90 字符，写作提交信息时以较短者自约通常最稳妥。合并后由于采用 squash-merge，PR 标题即为提交信息，因此 CONTRIBUTING.md 才会强调"把 PR 标题写对很重要"。

依赖管理：加依赖前与升级依赖的纪律

技能文档把依赖审查也纳入合并前门禁，核心立场是：每个依赖都是一项负债（liability）。加新依赖前依次回答五个问题：

1. 现有技术栈能否解决这个问题？（通常可以。）
2. 依赖有多大？（检查打包体积影响。）
3. 是否仍在积极维护？（看最近提交与未关闭 issue。）
4. 是否有已知漏洞？（跑npm audit。）
5. 许可证是什么？（必须与项目兼容。）

规则：优先使用标准库与既有工具，而不是引入新依赖。

升级依赖时另有四条纪律：

• 读 changelog，而不是只看版本号。Semver 只是维护者未必兑现的承诺。
• 一次只升一个依赖。批量升级一旦弄坏构建，你根本不知道是哪个包干的。
• 让测试说了算——升级前与后测试套件都要全绿，而不是"装上就完事"。
• 审查 lockfile diff 而不只是package.json；提交 lockfile，绝不手工编辑它。

供应链风险的优先级分诊则转交给security-and-hardening技能处理（其在仓库内还补充了用npm ci复现构建、警惕陌生包的postinstall脚本与 typosquat 攻击等卫生习惯）。

常见合理化借口对照表

文档用一张"借口 vs 现实"对照表，专治评审过程中最常见的自我说服。这些论点在人类与 Agent 评审场景中同样高频出现：

红旗清单：评审者要警惕的信号

技能文档汇总了一套"一票关注"的评审红旗：

• 没有任何评审就合并的 PR
• 只检查测试是否通过的评审（忽略其余各轴）
• 没有实际评审证据的 "LGTM"
• 安全敏感变更没有安全视角的评审
• "大到没法好好评审"的大 PR（应拆分）
• bug 修复 PR 没有回归测试
• 接受"我以后修"——它永远不会发生
• 只是搬移代码而没减少读者需记住概念数量的重构
• 新条件分支散落进无关代码路径（缺少抽象的信号）
• 自制 helper 与既有权威 helper 功能重复
• 不看 changelog 的"批量升依赖"PR

这些红旗与仓库协作规范互相印证：例如 CONTRIBUTING.md 明确把"提交未经人工评审的 AI 生成代码"列为不接受项，而 AGENTS.md 的自动触发规则也要求处理安全通告类事件时先取证再动手——安全敏感变更在 Penpot 生态中始终享有最高评审权重。

复核清单（Verification Checklist）

评审全部完成之后，文档要求对照以下清单收尾：

• 所有 Critical 问题已解决
• 所有 Required（无前缀）变更已解决，或被明确延后且附理由
• 测试通过
• 构建成功
• 验证过程有据可查（改了什么、如何验证的）
• 依赖升级已对照 changelog 审查、按包隔离、并经全绿测试套件验证

多模型评审模式：用不同模型对冲盲区

文档建议为同一变更引入不同模型的视角：

Model A 写代码 → Model B 评审 → Model A 处理反馈 → 人类做最终裁决

理由是：不同模型有不同的盲区。这与整套技能的设计哲学一致——评审的价值恰恰来自"另一双眼睛"，无论这双眼睛属于人类还是另一个模型。在 Penpot 仓库的上下文中，这与人机协作的最终把关责任是兼容的：模型可以无限迭代评审与修改，但合并与否的最终裁决始终保留给人类，CONTRIBUTING.md 所规定的"至少一次人工批准、默认 squash-merge"正是这套模式的落点。

相关联技能地图

• 评审计划/设计文档：走 plan-review 技能（六轴计划评审，其"建议的代码质量"轴直接引用 code-review 的标准）
• 评审代码/PR/diff：走本文讲解的 code-review 技能（两者由 review 命令按评审对象自动分发）
• 详细安全评审指引：见 security-and-hardening 技能
• 测试策略与 bug 复现测试要求：见 testing 技能
• 生成计划的上游技能：planner技能（.opencode 目录内）；提交与 PR 格式则由 AGENTS.md、CONTRIBUTING.md 与 scripts/check-commit 共同约束

小结

把code-review技能放回 Penpot 仓库语境看，它本质上是一份可执行的评审协议：用五轴统一"看什么"，用严重度五级统一"怎么说"，用七段式报告统一"怎么交"（含正面观察与最终裁定），用规模阈值与拆分策略统一"怎么拆"，再用红红旗清单与复核清单统一"什么时候放行"。这套协议同时约束人类与 AI Agent，最终服务于文档在开头就点明的那条批准标准——只要变更明显提升整体代码健康度，就让它通过；持续改进，而非追求不存在的完美。

【免费下载链接】penpotPenpot: The open-source design platform for Product teams that need scalable collaboration.项目地址: https://gitcode.com/GitHub_Trending/pe/penpot