Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:130971
Feedback-ID: ifab94697:Fastmail
Precedence: list
MIME-Version: 1.0
Date: Tue, 19 May 2026 16:54:24 +0200
To: "Gina P. Banyard" <internals@gpb.moe>,
 "Valentin Udaltsov" <udaltsov.valentin@gmail.com>
Cc: "php internals" <internals@lists.php.net>
Message-ID: <f4960fe4-dc03-4e99-b979-f15b9998628b@app.fastmail.com>
In-Reply-To: 
 <NqySiZ-sHKSjEjN1m4XGkxi71JQ2HH4uGTxzz1TsvA7F3LhH0pif0A6pI89UNvFOXJD6wp_JrALlBHP8yEipfVzKHbrC5eBjsjriz8W52Ts=@gpb.moe>
References: 
 <CAPyeZ4NcO_sZYe71Ef_nT0WtXfYhXpBBUFgzBgrrw_A+Oc3ZAg@mail.gmail.com>
 <NqySiZ-sHKSjEjN1m4XGkxi71JQ2HH4uGTxzz1TsvA7F3LhH0pif0A6pI89UNvFOXJD6wp_JrALlBHP8yEipfVzKHbrC5eBjsjriz8W52Ts=@gpb.moe>
Subject: Re: [PHP-DEV] Merging pull requests instead of closing them
Content-Type: multipart/alternative;
 boundary=9a89c52296b747a3ebd699733896d92bfe958fd0
From: rob@bottled.codes ("Rob Landers")

--9a89c52296b747a3ebd699733896d92bfe958fd0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

On Tue, May 19, 2026, at 14:31, Gina P. Banyard wrote:
> On Tuesday, 19 May 2026 at 13:04, Valentin Udaltsov <udaltsov.valentin=
@gmail.com> wrote:
>> Hi internals,
>>=20
>> I want to bring up something that's been bothering me since my contri=
bution <https://github.com/php/php-src/pull/13029>: many PRs in php-src =
are being closed instead of merged, with the changes pushed separately a=
s standalone commits.
>>=20
>> The problem is that GitHub marks these PRs as "Closed", not "Merged".=
 So from the outside =E2=80=94 and from the contributor's perspective =E2=
=80=94 it looks like the work was rejected. Their GitHub profile shows n=
o merged contribution, even though the code was accepted and is now in t=
he repo.
>>=20
>> Beyond statistics, it also disconnects the commit from the review dis=
cussion, and it's just confusing, especially for newer contributors who =
don't know the convention.
>>=20
>> I get that there might be reasons for this =E2=80=94 keeping a linear=
 history, concerns about merge commits, etc. But rebasing or squashing b=
efore merging solves all of that. Asking a contributor to rebase or squa=
sh is totally normal, and most people are happy to do it.
>>=20
>> Merging PRs is the standard across open source. It's how you signal t=
hat a contribution was accepted.
>>=20
>> So my question is: is there a way to make merging the default practic=
e in php-src? Has this been discussed before? If so, what were the block=
ers?
>=20
>=20
> Merging PRs that target master is common practice.
> However, when merging bug fixes our process is to merge into the lowes=
t branch and then up, something GitHub cannot do nor support, as sometim=
es we need to make changes to the fix in the merge up process.
> There has been discussion about this previously off list, but changing=
 the merge process is difficult and no-one has come to an agreement.
> However, 90% of you complain are issues with GitHub. We amend the comm=
it message to close the PR so that the link is preserved, and GitHub, if=
 it would be smarter, would be able to understand that it actually means=
 "merged" not "closed".
> I think relying on GitHub to provide useful statistics is just a fools=
 errand as it doesn't even consistently mark certain things as "reviews"=
 or not if you want to gather usage statistics.
>=20
> Best regards,
>=20
> Gina P. Banyard

Surely between everyone on this list ... we know people who work at GitH=
ub?

=E2=80=94 Rob
--9a89c52296b747a3ebd699733896d92bfe958fd0
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title></head><body><div>On Tue, May =
19, 2026, at 14:31, Gina P. Banyard wrote:</div><blockquote type=3D"cite=
" id=3D"qt" style=3D""><div style=3D"font-family:Arial, sans-serif;font-=
size:14px;"><div class=3D"qt-protonmail_quote"><div>On Tuesday, 19 May 2=
026 at 13:04, Valentin Udaltsov &lt;udaltsov.valentin@gmail.com&gt; wrot=
e:</div><blockquote class=3D"qt-protonmail_quote" type=3D"cite"><div dir=
=3D"ltr"><div><div>Hi internals,</div><div><br></div><div>I want to brin=
g up something that's been bothering me since <a href=3D"https://github.=
com/php/php-src/pull/13029" target=3D"_blank" rel=3D"noreferrer nofollow=
 noopener">my contribution</a>: many PRs in php-src are being closed ins=
tead of merged, with the changes pushed separately as standalone commits=
.</div><div><br></div><div>The problem is that GitHub marks these PRs as=
 "Closed", not "Merged". So from the outside =E2=80=94 and from the cont=
ributor's perspective =E2=80=94 it looks like the work was rejected. The=
ir GitHub profile shows no merged contribution, even though the code was=
 accepted and is now in the repo.</div><div><br></div><div>Beyond statis=
tics, it also disconnects the commit from the review discussion, and it'=
s just confusing, especially for newer contributors who don't know the c=
onvention.</div><div><br></div><div>I get that there might be reasons fo=
r this =E2=80=94 keeping a linear history, concerns about merge commits,=
 etc. But rebasing or squashing before merging solves all of that. Askin=
g a contributor to rebase or squash is totally normal, and most people a=
re happy to do it.</div><div><br></div><div>Merging PRs is the standard =
across open source. It's how you signal that a contribution was accepted=
.</div><div><br></div><div>So my question is: is there a way to make mer=
ging the default practice in php-src? Has this been discussed before? If=
 so, what were the blockers?</div></div></div></blockquote><div><span></=
span><br></div><div><br></div><div>Merging PRs that target master is com=
mon practice.</div><div>However, when merging bug fixes our process is t=
o merge into the lowest branch and then up, something GitHub cannot do n=
or support, as sometimes we need to make changes to the fix in the merge=
 up process.</div><div>There has been discussion about this previously o=
ff list, but changing the merge process is difficult and no-one has come=
 to an agreement.</div><div>However, 90% of you complain are issues with=
 GitHub. We amend the commit message to close the PR so that the link is=
 preserved, and GitHub, if it would be smarter, would be able to underst=
and that it actually means "merged" not "closed".</div><div>I think rely=
ing on GitHub to provide useful statistics is just a fools errand as it =
doesn't even consistently mark certain things as "reviews" or not if you=
 want to gather usage statistics.</div><div><br></div><div><div><div sty=
le=3D"background-color:rgb(255, 255, 255);"><span class=3D"font" style=3D=
"font-family:Verdana, sans-serif;">Best regards,</span></div><div style=3D=
"background-color:rgb(255, 255, 255);"><br></div><div><span style=3D"bac=
kground-color:rgb(255, 255, 255);"><span class=3D"font" style=3D"font-fa=
mily:Verdana, sans-serif;">Gina P. Banyard</span></span></div></div></di=
v></div></div></blockquote><div><br></div><div>Surely between everyone o=
n this list ... we know people who work at GitHub?</div><div><br></div><=
div id=3D"sig121229152">=E2=80=94 Rob</div></body></html>
--9a89c52296b747a3ebd699733896d92bfe958fd0--