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前端与导出器、Rustrender-wasm 渲染与 TypeScriptMCP、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-mergePR 标题即最终提交信息、评审期间用新增提交回应意见而非 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是否存在 N1 查询模式是否存在无界循环或不受约束的数据拉取是否有本应异步却在同步执行的操作UI 组件是否存在无谓的重复渲染列表端点是否缺少分页热路径上是否创建了大对象评审流程从理解意图到归类发现技能文档定义了五步评审流程理解意图Understand the intent——这个变更要达成什么实现的是哪份规格或任务先看测试Review tests first——测试揭示意图与覆盖率。它们测的是行为还是实现细节边界情况覆盖了吗评审实现Review the implementation——带着五个轴逐文件走查。给发现分类Categorize findings——每条评论都打上严重度标签。验证验证本身Verify the verification——跑了哪些测试构建通过了吗是否手工验证过UI 变更是否有截图第四步的严重度分级表格是整个评审能顺畅推进的关键基础设施前缀含义作者动作Critical:阻止合并安全漏洞、数据丢失、功能损坏High:必须修改合并前必须解决Medium:应当修复强烈建议但不阻塞Low:次要、可选作者可忽略——格式、风格偏好Suggestion:值得考虑非必需但能改善代码对每一条发现文档都要求描述触发失败的具体场景是何种输入、何种负载条件、何种时序或用户操作会触发问题。判断标准是——当输入为 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 行通常就是一个需要警惕的信号当一次改动实质性地撑大本已庞大的文件时应先做分解。文档给出四种拆分策略策略做法适用场景Stack堆栈先提交一个小变更在其基础上开启下一个顺序依赖By file group按文件组为需要不同评审者的文件组分开提交跨切面关注点Horizontal水平先建共享代码/桩再建消费方分层架构Vertical垂直按特性的垂直切片拆成多个小型全栈片段特性开发一条铁律把重构与功能开发分开。一个既做重构又加新行为的变更实际上是两个变更——应分别提交。这种一次一个逻辑变更的粒度要求也呼应了 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-commitPython 实现其检查器包括匹配 CI 正则允许:emoji: 大写主语且无结尾句点或Merge|Revert|Reapply前缀、主语不超过 90 字符、主语无尾句点、主语大写、主语与正文间空行、以及代码变更必须包含Signed-off-by:DCO。注意一个细节差异CONTRIBUTING 建议主语不超过 70 字符而 check-commit 脚本的硬性上限是 90 字符写作提交信息时以较短者自约通常最稳妥。合并后由于采用 squash-mergePR 标题即为提交信息因此 CONTRIBUTING.md 才会强调把 PR 标题写对很重要。依赖管理加依赖前与升级依赖的纪律技能文档把依赖审查也纳入合并前门禁核心立场是每个依赖都是一项负债liability。加新依赖前依次回答五个问题现有技术栈能否解决这个问题通常可以。依赖有多大检查打包体积影响。是否仍在积极维护看最近提交与未关闭 issue。是否有已知漏洞跑npm audit。许可证是什么必须与项目兼容。规则优先使用标准库与既有工具而不是引入新依赖。升级依赖时另有四条纪律读 changelog而不是只看版本号。Semver 只是维护者未必兑现的承诺。一次只升一个依赖。批量升级一旦弄坏构建你根本不知道是哪个包干的。让测试说了算——升级前与后测试套件都要全绿而不是装上就完事。审查 lockfile diff 而不只是package.json提交 lockfile绝不手工编辑它。供应链风险的优先级分诊则转交给security-and-hardening技能处理其在仓库内还补充了用npm ci复现构建、警惕陌生包的postinstall脚本与 typosquat 攻击等卫生习惯。常见合理化借口对照表文档用一张借口 vs 现实对照表专治评审过程中最常见的自我说服。这些论点在人类与 Agent 评审场景中同样高频出现合理化借口现实能跑就行足够了能跑但不可读、不安全或架构错误的代码会复利式累积技术债。我写的所以我知道它是对的作者看不见自己的假设。每次变更都需要另一双眼睛。我们以后再清理以后永远不会来。评审就是那道质量门——用它。AI 生成的代码大概没问题AI 代码需要更多而不是更少的审查。它即使错了也自信且貌似合理。测试过了所以是好的测试必要但不充分——测不出架构、安全或可读性问题。重构让它更干净了搬移复杂度不等于降低复杂度。读者仍需记住同样多的概念结构就没有改进。只是往这个文件加了一小段小 diff 照样会把文件推过健康体积线把分支硬接在无关流程上。只是升个版本而已升级是一次你没写的行为变更。去读 changelog。我把所有东西一个 PR 升级完批量升级掩盖了到底是哪个包弄坏了构建。一次一个。虽然重复了但只有两处两处会变成三处、五处。趁副本尚未分叉现在就提取。这个抽象面向未来YAGNI。删除投机性泛化——第三次出现时才泛化不是第一次。它很巧妙但很高效巧妙是对可读性征税。如果它需要注释才能看懂就简化它。红旗清单评审者要警惕的信号技能文档汇总了一套一票关注的评审红旗没有任何评审就合并的 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创作声明：本文部分内容由AI辅助生成（AIGC），仅供参考