Cataclysm-DDA PR 评审实战指南：从检查清单到合并决策

Cataclysm-DDA PR 评审实战指南从检查清单到合并决策【免费下载链接】Cataclysm-DDACataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.项目地址: https://gitcode.com/GitHub_Trending/ca/Cataclysm-DDA导读本文是 Cataclysm: Dark Days AheadCDDA项目仓库内 doc/reviewing_PR_guide.md 的完整解读与实战扩展。CDDA 是一个回合制末日生存游戏其代码库规模庞大src/下数百个 C 源文件data/下数千个 JSON 数据文件日常合并的 Pull RequestPR数量众多而负责合并的志愿者人手有限。本文讲解该项目官方定义的 PR 评审流程、7 条核心检查清单、需要第二意见的警示信号以及 Discord 协作机制读完你既能作为评审者按清单逐项审查他人的 PR也能作为提交者自查自己的 PR 是否达到可合并标准并了解仓库 CI 与自动化工具如何辅助这一流程。一、背景与目标为什么 CDDA 需要标准化 PR 评审随着 CDDA 项目不断增长项目管理层Project Management的少数志愿者越来越难对每个合并进来的改动保持持续关注。doc/reviewing_PR_guide.md开篇点明了多年实践中的两种典型失控方式当核心开发者因现实原因暂时离开时要么 PR 积压成山要么少量本不该被合并的改动混进了主干。后一种情况的代价是必须回退revert那些由热心且善意的贡献者完成的成果这是项目方极力避免的坏结果。为此该文档确立了两个目标为复杂 PR 制定一套标准化的评审流程与检查清单借助上述流程提供一种更轻松地判断PR 是否已具备合并条件的方法。需要特别强调的是这套流程并非对所有 PR 强制要求项目方也不打算让它变成强制约束。它的定位是帮助贡献者与协作者collaborators知道如何评审并批准一个复杂 PR从而让拥有合并权限的人可以更高效地逐一评估。同时提交者也可以把这份清单当作自我检查的指南。文档明确告诫不要用它来对项目规则钻牛角尖rules lawyer——一个 PR 即便违反了所有这些条目仍有可能被合并。它们是指南guidelines不是铁律。二、如何使用本文档文档给出了三种使用方式由使用者自行选择在浏览器中打开本文档评审时随时对照通读并内化其中的原则评审时不直接引用最稳妥的方式把检查清单逐条复制粘贴到自己的评审意见中逐项勾选核对。如果你不确定如何评审一个 PR最后一种方式是相当安全的选择。这套做法旨在统一评审中呈现给合并者的信息让评审意见包含合并决策最需要的内容。三、PR 评审检查清单PR Review Checklist这是整份指南的核心骨架。以下 7 条即官方清单原文每条在后续小节中都有详细展开PR 聚焦于单一领域。PR 少于约 300 行 C 或约 1000 行 JSON。PR 未使用项目之外的材料。改动与 PR 描述一致。问题的解决方案可维护、可扩展。游戏平衡性改动有充分证据支持 / 获得领导层批准。已检查失败的 CI 测试且与 PR 无关。再次强调这些是如何评审 PR的建议而非合并的硬性规则。一个 PR 可以违反其中任何一条而不一定是问题但一般来说如果评审者发现多条不满足这通常预示该 PR 需要项目内更高权限的人再审视一遍。3.1 第 1 条聚焦单一领域Focused on a single areaPR 必须聚焦于单一改动领域。虽然反正我都改到这里了顺手把这个也修了的冲动很常见但这会让 PR 难以阅读更糟的是如果引入了意外 bug会让 bug 追踪变得非常困难。这是最重要的一条建议文档认为几乎不存在合理的违反借口——尽管连部分核心开发者偶尔也会犯这个毛病。需要注意这不意味着一个 PR 不能触及大量文件或一次改动很多东西有时这是不可避免的。同时项目方大多数时候不希望 PR 为尚未被使用的功能提前添加代码因此建议 PR 尽量一次把功能做到可运行的程度至少实现其中的一部分。对于大型重写overhaul型 PR项目方通常更希望尽可能拆分为小块。文档给出的例子是重写怪物攻击机制第一个 PR实现新攻击代码但保留旧攻击代码只改动经典僵尸的攻击后续 PR逐个改动其他僵尸的攻击后续 PR改动其他怪物的攻击最终 PR删除旧的攻击代码。这样拆分可以在各 PR 之间留出时间追踪变化、发现问题并且让评审者能同时审查新攻击的代码实现和怪物攻击数值的平衡性。如果这一切都塞进一个 3000 行的 PR几乎不可能仔细梳理每只被改动的怪物并确认其新攻击数值是否合适。当然与所有指南一样这并不意味着大 PR 就该立即关闭、放弃。如果一个大 PR 实在找不到合理的拆分方式它很可能是没问题的——只是需要更高层级的评审者来审查。3.2 第 2 条长度限制Length restrictions长度限制只是建议但非常重要。超过该规模的 PR 有很高概率违反第 1 条聚焦单一领域。例外情况包括不违反第 1 条且超长有正当理由例如对大量文档进行的简单自动化修改改动被整洁地拆分成有组织的提交commits且合并时不会被 squash 掉地图生成mapgen改动几乎总是豁免因为地图数据本身极其庞大——但如果你还附带了很多其他改动建议先把地图改动单独拆成一个 PR。这条指南的出发点与第 1 条类似超长 PR 引入错误的概率更高且错误一旦出现就更难定位。评审者和修 bug 的人都是人改动块越长找到问题引入点就越困难。同时文档也承认由于历史教训有些改动确实无法拆分为多个 PR只要有充分理由就没问题。特别是项目方不喜欢死代码dead code如果一个 PR 因为实现新系统 添加使用该系统的地图或怪物而变长这通常比先加系统、后续 PR 再加地图/怪物更可取。不符合该条的 PR 可能因为难以评审而需要更长时间才能合并但最终仍会被处理。3.3 第 3 条外部来源材料Outside sourced materialCDDA 以 CC-BY-SA 4.0 许可证发布仓库根目录的 LICENSE.txt 与 CONTRIBUTING.md 中有明确声明。除非你非常确定自己完全理解该许可证的运作方式否则不应批准任何使用项目之外材料的 PR——哪怕是名字、明显的引用等也不行。文档要求不要接受别人对该许可证如何运作的解释不要假设某材料小到微不足道不要假设材料属于合理使用fair use直接标记为不满足此标准即可。违反这条指南不等于 PR 被拒绝只是需要更仔细地审查。核心开发者已经被迫对此相当专业项目方希望规则被一致地执行因此一旦被标记他们会介入评估是否合规。从仓库实际内容看许可证合规是硬性要求而非空话CONTRIBUTING.md 的Licensing and Authorship一节明确要求所有贡献必须由提交者本人创作从其他 fork 移植改动且保留署名等少数情况除外并明确禁止使用 LLM如 ChatGPT生成的内容——包括代码、配置、Issue/PR 文本、研究及测试结果因为这类内容没有作者署名来源在法律和道德上都存疑。移植他人代码时必须保持原始作者署名且需尽职确认被移植内容由人类而非 LLM 创作。这条与评审指南第 3 条形成呼应评审者在批准 PR 前实质上也需要对这些署名与来源问题保持警觉。3.4 第 4 条改动与描述一致Changes consistent with the descriptionPR 描述不一定需要事无巨细地列出每一行改动但读完描述应该能清楚知道改动里会看到什么。描述中不应有惊喜如果描述写完后某些内容又改了必须同步更新描述。文档给出了一个很实用的自检方法想象一年后你回到这个 PR试图回忆它包含什么、判断它是否可能造成某个 bug——如果仅凭 PR 描述就能做到这一点描述就合格了。如果后来的人必须阅读代码改动才能知道自己是否找对了地方那说明描述需要改进。3.5 第 5 条可维护、可扩展Maintainable and extendible这一条相当依赖评审者的判断。花哨janky的解决方案是指试图用取巧手段绕过代码库限制的方案。文档指出这类方案在 JSON 贡献中很常见尤其是 EOC即 Effect-on-Condition 相关逻辑但在 C 中也非常普遍。有些时候它们可以被接受但以奇怪方式利用代码的做法很可能在别的东西开始依赖该代码时立刻出问题。如果 PR 改动的是游戏其他部分很可能依赖的内容这条就更加重要。3.6 第 6 条平衡性改动要有依据Balance changes have been justified与第 4 条一样这某种程度上是个人判断。改动少量物品的耐久度、或一批东西的生成频率不需要大量论证。文档举的对比很直观没有任何房子有烤面包机所以我给大多数房子加了烤面包机——这是很好的改动几乎不需要来源支撑游戏区域应该有更多中世纪盔甲——这是对平衡的重大改动需要更多来源或者退而求其次需要高级项目负责人如 Kevin、Zhilkinserg、KorGgent、Erk明确放行或得到多位经验丰富的开发者Discord 上的绿名认可有时多位协作者的意见也足够。这一点在涉及既往争议性话题时尤其突出例如弓和太阳能以及科幻/奇幻内容cbms 义体、突变、传送门等。文档用一句话概括了这条原则非凡的主张需要非凡的证据普通的主张需要普通的证据。如果说硬件店应该刷出管道胶带不需要太多来源但如果说仅凭太阳能电池板就能给电动汽车提供全部电力那就需要非常充分的证据——毕竟现在还没有人量产太阳能汽车。3.7 第 7 条失败的 CI 测试与 PR 无关Failing CI tests arent related每个 PR 都会运行大量 CI 测试。要给 PR 通过性评审你必须看过这些测试结果。文档坦诚地说我们也都懒惰而且也是人知道无视它们很诱人但请务必不要这样做。CDDA 的 CI 管线在仓库 .github/workflows 下有大量实际落地评审时可以对照这些工作流理解有哪些测试在跑matrix.yml通用构建矩阵覆盖多平台Linux/macOS/Windows/Android与多个编译器版本如 clang-13、clang-18、gcc-9、gcc-14并在不同组合中开启 TILES、SOUND、RELEASE、CMAKE、LOCALIZE0、LTO、LIBBACKTRACE、SANITIZEaddress/undefined等特性开关还会启用 Magiclysm 等关键模组运行测试。文件头部的注释矩阵直观列出了各构建的覆盖目标评审者可据此确认自己的改动在哪些组合下被验证过astyle.yml对**.cpp、**.h、**.c等文件执行make astyle-check失败时自动make astyle-fast格式化并展示git diff --color修正差异。这与 CONTRIBUTING.md 中代码风格由 astyle 全库强制执行的说明一致json.yml对**.json执行make style-all-json-parallel负责 JSON 数据的风格校验clang-tidy.ymlclang-tidy 22 静态分析按PR 直接改动的文件 / src 目录 / 其余文件三个子集拆分运行并构建自定义插件libCataAnalyzerPlugin.so实现项目定制的检查规则pr-validator.yml校验 PR 描述是否包含合规的#### Summary行使用正则(\n|^)#### Summary\s{0,3}(SUMMARY:\s)?(None|((Features|Content|Interface|Mods|Balance|Bugfixes|Performance|Infrastructure|Build|I18N) .)){0,3}\s*(\n|$)匹配忽略大小写——这直接对应下方提交者自查部分提到的 Summary 硬性要求。除了这些工作流CONTRIBUTING.md 还介绍了本地测试方式源码树内置测试套件位于 tests 目录任何对游戏源码的改动后都应运行测试make会构建测试可执行文件tests/cata_test无参数运行会执行整个测试套件--help可查看运行选项也可用make check一键执行。评审者如果对 PR 中的测试结果存疑可以在本地复跑验证。四、需要第二意见的警示信号Indicators to call for a second opinion以下观察点不属于评审清单但它们是应该让不止一位有合并权限的人参与评估的合理理由。文档特别强调这些信号不意味着你的 PR 不会被合并只是根据经验它们往往与日后出问题的 PR 相关联PR 作者抗拒对其 PR 提出的修改建议尤其是来自协作者及以上层级的建议大规模全面重写型 PR却没有对应的 Issue 或类似文档讨论重写的设计方向与计划仅以玩家需要/想要为理由的改动缺乏更深层依据。文档指出与普遍认知相反改善玩家体验本身是可取的但经验表明仅仅问人会因受访对象不同而得到迥异的答案以此为依据的改动很少具备统计学严谨性用新计算推翻既往有争议的调整。这一点可能很难识别例如有人基于自己的研究上调弓的伤害——这类话题已被研究得死去活来新贡献者几乎不可能虽然并非绝对不可能了解此前所有相关工作因此合并门槛非常高触及任何政治热点话题的改动。这类内容经常是钓鱼引战troll bait凡是涉及骄傲旗、跨性别/同性恋权利、移民、现实政治人物的内容都应密切监视同样过于猎奇/出格edgy的内容——尤其是与食人、NPC 奴役等相关的内容需要被非常密切地监视应景幽默、迷因、流行文化引用等。它们常常出于好意被加入但保质期通常比 PR 合并完成还短即使是要进入 Crazy Cataclysm 之类的模组也应带着极大的保留态度看待。五、Discord 的角色体系与协作机制项目方并不要求所有贡献者活跃于 Discord但官方 Discord 服务器见仓库根目录 README.md是最快、最可靠的途径用来找对人讨论 PR 遇到的问题或作为评审者为 PR 吸引注意、询问这些问题。尤其是在 #development 频道围绕这些话题提出问题基本不会惹麻烦。指南中反复提到lead devs核心开发者developers开发者collaborators协作者指的是其评审意见格外有帮助的人群。Discord 上的角色标识规则如下金色名字senior devs高级开发者成员列表顶部的senior devs分组下即核心开发者团队绿色名字developers开发者绝大多数绿名属于这一级别尤其在各自负责的代码领域话语权几乎与高级开发者持平蓝色名字collaborators协作者单个协作者不是 PR 能否被接受的最终决定者但该角色用于标识总体上理解项目如何运作、值得信任的人紫色名字contributors贡献者可以征求意见并给出意见但他们对项目没有特别额外的权威参考时需保持审慎文档甚至建议即便是高级别人员的意见也应带着一点保留看待——只有 Kevin 是最终权威。无论 Discord 角色如何任何愿意的人都可以也应该为别人的 PR 提供评审贡献者完全可以使用这份指南进行自己的评审。六、提交者视角仓库自动化如何呼应这份评审指南这份指南不只是给评审者用的。如果你是要提交 PR 的贡献者仓库中的实际机制已经把指南的许多精神固化成了自动化约束PR 模板强制结构仓库 .github/pull_request_template.md 要求每个 PR 必须包含#### Summary一行式摘要、#### Purpose of change、#### Describe the solution、#### Describe alternatives youve considered、#### Testing、#### Additional context等章节。其中 Summary 是强制项格式为Category description类别只能是 Features、Content、Interface、Mods、Balance、Bugfixes、Performance、Infrastructure、Build、I18N或直接写None表示不进入 changelog模板本身还提示大多数 PR 少于 500 行改动——与评审指南第 2 条的长度精神一脉相承。各类别的详细含义见 doc/CHANGELOG_GUIDELINES.md。CI 自动拦截不合规 PR如上一节所述pr-validator.yml 会用正则校验 Summary 行非法 Summary 的 PR 无法合并json.yml 与 astyle.yml 分别保证 JSON 与 C 风格一致——这直接支撑评审指南第 5 条可维护、可扩展一致的风格与结构是长期可维护性的基础。按路径自动分配评审者.github/reviewers.yml 为data/mods/**下的各模组目录、src/widget.*、src/ui_manager.*、data/raw/keybindings等特定路径映射了对应的熟悉该领域的评审者如 Magiclysm 对应 KorGgenT、**/magic*.cpp对应 KorGgenT 等。这与指南中在各自负责的代码领域开发者话语权几乎与高级开发者持平的协作理念相吻合也让第二意见机制在组织层面有了落点。评论指令机器人.github/comment-commands.yml 记录了维护者/志愿者可用的评论指令如/confirm确认 bug、/duplicate标记重复、/good-first-issue、/retry重试 CI 任务等这是评审与维护流程的辅助工具评审者遇到 CI 偶发失败时可用/retry重跑。七、结语CDDA 的 PR 评审文化可以概括为清单是拐杖不是枷锁。doc/reviewing_PR_guide.md提供的 7 条清单把聚焦单一领域、控制规模、来源合规、描述一致、方案可维护、平衡有依据、CI 无关联失败变成了一套可复制的评审框架而第二意见警示信号则提醒社区警惕那些容易在日后演变成问题的 PR 模式。配合仓库内 PR 模板、Summary 校验、构建矩阵与静态分析等一系列 CI 工作流这份指南既是评审者的操作手册也是贡献者的自检清单——最终目标始终是减少不必要的回退让热心贡献者的工作能被安全、稳妥地合并进这个末日世界。【免费下载链接】Cataclysm-DDACataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.项目地址: https://gitcode.com/GitHub_Trending/ca/Cataclysm-DDA创作声明：本文部分内容由AI辅助生成（AIGC），仅供参考