Hi Valentin
GitHub marks PRs as "Merged" under either of these two conditions:
* The PR is merged via GitHub UI
* The latest commit of the source branch is pushed to the target branch
As explained by Gina, most of us already merge most PRs that target
master via GitHub UI, unless they require additional changes like a NEWS
entry. When merging commits manually, the source branch usually requires
a rebase (as feature/bugfix branches are merged via fast-forward merge).
Additionally, we frequently amend some information to the commit
message, like adding a reference to the PR. Either of those change the
commit SHA and will break GitHub's ability to track the merge status of
the PR and leave the PR open. To fix this, we usually add "Closes
GH-xyz" to the commit message to auto-close the PR. Some also just close
the PR manually with a link to the merged commit. The catch is that the
status of the PR will be "Closed" rather than "Merged".
To make GitHub pick up on the merge, the source branch would need to be
force-pushed after the rebase so that the fast-forwarded commit once
again matches that of the latest commit of the source branch. However,
this also triggers CI and would incur a non-trivial amount of fully
redundant resource usage. That's the main reason this has not been
implemented (apart from the additional step).
The benefit OTOH is that the PR changes would be fully in sync with what
has actually been merged, which is something Tim has argued for in the
past. If we can find a way to avoid the additional CI resource usage,
I'd be happy to implement these changes.
Ilija
On 19.05.26 14:00, Valentin Udaltsov wrote:
Hi internals,
I want to bring up something that's been bothering me since my
contribution <https://github.com/php/php-src/pull/13029>: many PRs in
php-src are being closed instead of merged, with the changes pushed
separately as standalone commits.
The problem is that GitHub marks these PRs as "Closed", not "Merged".
So from the outside — and from the contributor's perspective — it
looks like the work was rejected. Their GitHub profile shows no merged
contribution, even though the code was accepted and is now in the repo.
Beyond statistics, it also disconnects the commit from the review
discussion, and it's just confusing, especially for newer contributors
who don't know the convention.
I get that there might be reasons for this — keeping a linear history,
concerns about merge commits, etc. But rebasing or squashing before
merging solves all of that. Asking a contributor to rebase or squash
is totally normal, and most people are happy to do it.
Merging PRs is the standard across open source. It's how you signal
that a contribution was accepted.
So my question is: is there a way to make merging the default practice
in php-src? Has this been discussed before? If so, what were the blockers?
Thanks,
Valentin Udaltsov
