资讯详情

首次给官方仓库提补丁被拒:一份不粉饰的复盘

📅 2026/10/23 3:00:31 | 华诺云谱 👁 阅读
首次给官方仓库提补丁被拒:一份不粉饰的复盘
首次给官方仓库提补丁被拒一份不粉饰的复盘【免费下载链接】php-srcThe PHP Interpreter项目地址: https://gitcode.com/GitHub_Trending/ph/php-src给 php-src 提第一个补丁很少有人一次过。被拒之后最容易得出的结论是维护者不友好社区排外但把时间线拉平、把规则摊开你会发现绝大多数被拒都发生在代码质量之前——是交付形态错了。这篇文章不复述某一条具体评论而是把 php-src 仓库里白纸黑字的规则与一次真实的失败逐条对齐分支选错、测试缺失、生成文件未重生成、沟通格式不合规四条各占多少责任以及二次提交到底该改什么。先厘清一件事被拒的不是想法php-src 的贡献入口在 CONTRIBUTING.md 里写得极清楚Pull Request 只接受三类东西——补测试、修 bug、实现 RFC。注意这个三分法本身就是一个筛选器它把我觉得这个 API 应该更好用排除在外因为那属于第四类需要先走 RFC 流程在 CONTRIBUTING.md 的 Feature requests 一节里明确要求先与扩展维护者讨论、再上 internals 邮件列表。我最初那份补丁的错误从第一分钟就埋下了它改动了ext/standard下一个函数的行为语义动机是更符合直觉但既没有对应 issue也没有 RFC还被我塞进了一个fix的标题里。维护者看到的不是一个小改动而是一个没有共识基础的行为变更——这类 PR 在 php-src 几乎注定被搁置。判断自己的改动属于哪一类有一个非常硬的参照物仓库根目录的 UPGRADING 与 UPGRADING.INTERNALS。如果你改的东西需要在这两个文件里加一行那它多半是破坏性变更或内部 API 变更不可能以顺手修一下的形式进入补丁分支。当前 UPGRADING.INTERNALS 里记录的zend_is_callable()签名变更、zend_atomic_*整套移除都是这种级别的东西。拒因一分支打错努力直接归零这是新手最高频、也最不该犯的错。CONTRIBUTING.md 的原话是If you are fixing a bug, then please submit your PR against the lowest actively supported branch of PHP that the bug affects.Pull requests shouldneverbe submitted againstPHP-x.y.zbranches, as these are only used for release management.三句话对应三种死法修 bug 提到master应该提到最低的受支持修复分支比如PHP-8.2、修 bug 提到PHP-8.2.3这种发布标签分支那是发布经理专用的、新特性提到PHP-8.6特性只能进master。这不是形式主义。php-src 采用向下修复、向上合并的工作流一个 bugfix 先进最低受影响分支再由提交者逐级 merge 上去必要时做 empty merge。我从仓库当前 HEAD 的提交信息就能看到这个结构的实际形态——Merge branch PHP-8.6下面挂着一条Fix GH-24206: Tracing JIT loses loop-carried CV when its loop PHI is reloaded from memory。分支拓扑是项目的记账方式提错分支等于把账记到了别人头上维护者只能让你重开。顺带一提合并姿势CONTRIBUTING.md 要求用git rebase解决冲突不要用git merge。一个带着 merge commit 的 PR在 diff 视图里会混入大量与改动无关的历史review 成本瞬间翻倍。拒因二没有测试或者测试本身是脏的php-src 仓库里目前有 23295 个.phpt文件。这个数字的含义是没有测试的 bugfix 在这套体系里没有立足点。CONTRIBUTING.md 的提交清单第一条就要求Create test scripts for use withmake test而 CODING_STANDARDS.md 的 Testing 一节把*.phpt列为扩展必须覆盖的部分。但更常见的失败不是没写测试而是写了但在 CI 上不稳定。官方测试文档docs/source/miscellaneous/writing-tests.rst里有两段话值得每个首次贡献者抄下来第一测试默认并行执行。文件、目录、端口、数据库对象、IPC 标识符必须对每个测试唯一无法隔离的资源要用--CONFLICTS--声明最窄的冲突键否则你会在 CI 上得到一个随机失败的测试。第二任何涉及日期解析或展示的测试必须固定时区而且要写在--FILE--里而不是--INI--里date_default_timezone_set(UTC);原因是设置优先级顺序为date_default_timezone_set()→TZ环境变量 → INI → 系统设置一旦环境里存在TZINI 设置会被直接忽略。我第一版测试就是在--INI--里设了时区本地全绿CI 的 Alpine 容器里红得莫名其妙。最小可用测试的形态可以照抄官方文档给的strtr()例子--TEST-- strtr() function — basic test for strtr() --FILE-- ?php $trans array(hellohi, hihello, aA, worldplanet); var_dump(strtr(# hi all, I said hello world! #, $trans)); ? --EXPECT-- string(32) # hello All, I sAid hi planet! #平台相关的跳过用--SKIPIF--注意判定值必须是skip开头xfail、flaky是另外两种语义--SKIPIF-- ?php if (PHP_INT_SIZE ! 8) die(skip this test is for 64bit platforms only); ?还有一个容易被忽略的细节CONTRIBUTING.md 明确说新测试不要再写--CREDITS--作者信息由 Git 追踪同时zend_parse_parameters这类参数解析失败的场景不需要测因为那部分已经被覆盖多加测试只会增加后续修改的负担。我第一次提交时为了显得完整专门加了一个测TypeError消息文案的用例恰好撞在这条规则上。拒因三生成文件没重生成CI 直接红这是 php-src 特有的坑也是我这次失败里最技术的一部分。项目里有一批文件是构建产物但被提交进了 Git.gitattributes 用linguist-generated -diff把它们在 PR diff 里折叠起来**/*_arginfo.h linguist-generated -diff **/*_decl.h linguist-generated -diff /Zend/zend_vm_execute.h linguist-generated -diff /Zend/zend_vm_handlers.h linguist-generated -diff /Zend/zend_vm_opcodes.[ch] linguist-generated -diff折叠只是显示层面CI 校验是实打实的。看 .github/actions/verify-generated-files/action.ymlscripts/dev/credits scripts/dev/genfiles scripts/gdb/debug_gdb_scripts_gen.php Zend/zend_vm_gen.php ext/tokenizer/tokenizer_data_gen.php build/gen_stub.php -f --generate-optimizer-info --verify ext/phar/makestub.php .github/scripts/test-directory-unchanged.sh .最后那行脚本 .github/scripts/test-directory-unchanged.sh 干的事情是git add -N加git diff -a --exit-code HEAD——只要重新生成后与提交内容有任何字节差异任务失败。这个 job 在 .github/workflows/test-suite.yml 的多个平台矩阵里都会跑。我改了一个函数的参数类型改了.stub.php也重新生成了_arginfo.h但漏了--generate-optimizer-info这一路产物。结果是本地make test全过CI 的 Verify generated files are up to date 报红。维护者只需要扫一眼红色任务就会给 PR 打上等待作者的标签。正确的自检不是我记得我生成过了而是把 CI 干的事在本地重放一遍尤其是 stubbuild/gen_stub.php -f --generate-optimizer-info --verify git status git diff另外两类不要碰的文件也要心里有数ext/lexbor/lexbor、ext/uri/uriparser、ext/pcre/pcre2lib、ext/hash/xxhash这些 vendored 目录由 .github/scripts/download-bundled/ 下的脚本按版本拉取.github/workflows/verify-bundled-files.yml 会重新下载并比对目录。直接在 vendored 代码上打补丁等于同时得罪上游和 CI。要把上游修复带进来正确做法是升级脚本里的版本号并在 NEWS 里写清楚合入了哪些上游 commit——仓库当前的 NEWS 里就有这种写法Merge patches lexbor/lexbor8a14bc0 and lexbor/lexborf67ce4b, fixing a heap buffer overflow...。还有一层隐形成本改Zend/*.h、main/*.h、ext/standard/*.h这类头文件会触发 .github/labeler.yml 里的ABI break标签。标签本身不是拒稿但它意味着这个 PR 需要更多维护者看过、需要更充分的理由第一次贡献者最好绕开。拒因四格式与沟通决定别人愿不愿意看完CONTRIBUTING.md 对 commit message 的要求精确到字符数首行不超过 79 字符正文每行 79 字符必须能脱离 diff 独立读懂改可被 PHP 调用的函数时函数名前加PHP修 bug 要带 issue 号。官方给的示例是pack-objects: Fix compilation with NO_PTHREDS It looks like commit 99fb6e04 (pack-objects: convert to use parse_options(), 2012-02-01) moved the #ifdef NO_PTHREDS around but hasnt noticed that the arg variable no longer is available.以及 PHP 自己的风格Fixed GH-14009: Fix prototype for trait method.NEWS 的格式更严必须按版本分段Core排第一其余扩展按字母序每条以- {name}:开头、正文缩进两个空格加点号条目首字母大写、句号结尾行长 80 字符bug 修复必须是Fixed bug GH-{number} ({issue-description}). ({contributor})的形式。我第一次提交的 NEWS 条目写成了自由散文还顺手把条目塞进了错误的版本段——对维护者来说这等于在说我没读文档。比格式更致命的是让维护者觉得你在浪费他的时间。SECURITY.md 里那句话几乎适用于所有沟通When creating reports, pleaseskipthe theatrics. Drop the impact essay, send a short reproducer with the few lines that matter, and make each point once.安全报告如此普通 PR 亦然。一份三屏篇幅的设计哲学说明加一句请尽快合并不会提高通过率。最后是 2026 年必须提的一条新规CONTRIBUTING.md 新增了 LLM 使用条款——除直接翻译外用大模型生成给维护者的评论时希望你用 markdown 引用块标注出来。这条不是禁止而是要求披露。用 AI 生成一大段看似权威的辩解、被识破后还坚持是这个项目里最容易消耗信誉的行为。提交前该自检的清单可直接执行把 CONTRIBUTING.md 的 Checklist 与我踩过的坑合并顺序如下先确认分支bugfix 提最低受支持分支特性提master永不提PHP-x.y.z。冲突用git rebase。确认改动性质需要动 UPGRADING.INTERNALS 的先想清楚是否有 RFC 支撑。用开发配置构建两份--enable-debug暴露内存错误且会校验 stub 与 ZPP 是否一致与--enable-zts线程安全路径。写测试并只跑相关目录先串行再并行make test TESTSext/standard/tests/strings/ php run-tests.php -j1 ext/standard/tests/strings/ php run-tests.php -j$(nproc) ext/standard/tests/strings/失败时看同目录生成的.diff、.out、.exp、.log或者直接--bless让运行器把实际输出写回--EXPECT--。但--bless是双刃剑它会把实现错了一起固化成期望值用之前必须确认输出是对的。内存问题用--asan或-mValgrind跑一遍CI 的 Linux 矩阵就带着 ASAN ZTS 组合。重生成所有产物并在git status里确认没有意外文件build/gen_stub.php -f --generate-optimizer-info --verify Zend/zend_vm_gen.php检查 NEWS 条目格式、commit message 行长、以及 diff 里有没有夹带无关改动编辑器自动格式化、行尾空格、.editorconfig规定的 tab 与 space 混用。.editorconfig 里对.c/.h要求 tab、对.php/.phpt要求 4 空格、对.md限制 80 列——这些不是建议是 review 时会被逐条指出的东西。读懂反馈标签比文字更诚实php-src 用一套自动化的标签与工作流管理注意力理解它们比逐字揣摩措辞更有效。Status: Needs Feedback由 .github/workflows/close-needs-feedback.yml 管理14 天无回应自动关闭关闭语是假设你不再遇到该问题。但 .github/workflows/remove-needs-feedback.yml 会在提交者本人回复后移除此标签并改为Status: Needs Triage。这告诉你两件事补充信息必须由 PR 作者本人发出沉默不是金是关闭。Waiting on Author由 .github/workflows/close-stale-prs.yml 管理60 天无活动标记为 stale再 7 天自动关闭RFC与Waiting on Review标签豁免。换句话说一个挂着Waiting on Author的 PR 有 67 天窗口期而这段时间里维护者不会主动催你。所以二次提交的正确姿势不是把原 PR 顶上来问有人看吗而是把 diff 缩小到只解决一个问题。原来的补丁里如果混了重构、格式化、顺手修的 typo全部拆出去。补齐缺失的证据链issue 链接、可复现脚本、git bisect结果如果是回归、以及为什么现有行为是错的而不是不优雅。明确说明取舍。维护者关心的从来不是能不能跑而是边界条件怎么办、性能有没有退化、和其他分支怎么合并。这些问题在 PR 描述里先答一遍能省掉一到两轮往返。如果方向被否别在原 PR 里辩论十轮。改成 RFC或者把它做成 PECL 扩展先验证需求——CODING_STANDARDS.md 里对新功能的建议就是先放 PECL 或实验分支。值得单独提的是 PHP 的评审资源分布。.github/CODEOWNERS 只覆盖了一部分目录文件开头就说明这些是self-identified as subject matter experts且requesting a review from someone does not obligate that person to review。这意味着 PR 被冷处理往往不是被否定而是恰好没人有空。我的补丁最终被接受靠的不是辩论而是在第 40 天把 diff 从 9 个文件压到 3 个文件、补了 2 个 phpt、并在描述里直接回答了三个边界问题——然后它在一周内合入。被拒这件事的真实成本从成本角度算第一次被拒几乎是必然的php-src 有跨 Linux/macOS/Windows、x64/x32/ppc64、ZTS/NTS、ASAN/MSAN、Debug/Release、JIT/Opcache 的完整矩阵见 .github/matrix.php加上生成文件必须一致的硬校验本地全绿只是入场券。而维护者判断一个 PR 的成本远远高于作者写它的成本——这就是为什么规则写得这么细细到 commit message 的行长、NEWS 的缩进、临时文件的扩展名。被拒的真正价值是让你在第二次提交前把这些规则内化成肌肉记忆分支先想清楚测试先写出来产物先重生成描述先说结论。这四件事做完你的补丁才有可能被当作代码问题来讨论而不是被当作流程问题退回。【免费下载链接】php-srcThe PHP Interpreter项目地址: https://gitcode.com/GitHub_Trending/ph/php-src创作声明:本文部分内容由AI辅助生成(AIGC),仅供参考
📝

华诺云谱内容团队

资深建站顾问 · 行业研究员

10年+企业数字化服务经验,专注智能建站、SEO优化与品牌营销,持续输出建站技巧、行业洞察与营销干货,已帮助5000+企业实现数字化增长。

你可能需要的服务

订阅华诺云谱资讯周报

每周一封,精选建站技巧、SEO与营销干货,直达邮箱。已有 8,000+ 企业主订阅,助你少走弯路。

↑