Hi all!
I have noticed that the CODEOWNERS of php-src are apparently not synced
for all active branches[1][2]. However, it seems to me that this
doesn't make sense; if someone provides a PR against some lower branch,
the most recent CODEOWNERS may not be notified, and modifications might
be send to possibly inactive maintainers. This is particularly
confusing if the base branch of an PR will be changed; all over sudden
new reviews may be requested.
So I suggest to sync CODEOWNERS for all active branches (and maybe even
security branches).
What do you think?
[1] https://github.com/php/php-src/blob/PHP-8.2/.github/CODEOWNERS
[2] https://github.com/php/php-src/blob/master/.github/CODEOWNERS
Cheers,
Christoph
Hi Christoph,
Hi all!
I have noticed that the CODEOWNERS of php-src are apparently not synced
for all active branches[1][2]. However, it seems to me that this
doesn't make sense; if someone provides a PR against some lower branch,
the most recent CODEOWNERS may not be notified, and modifications might
be send to possibly inactive maintainers. This is particularly
confusing if the base branch of an PR will be changed; all over sudden
new reviews may be requested.So I suggest to sync CODEOWNERS for all active branches (and maybe even
security branches).What do you think?
[1] https://github.com/php/php-src/blob/PHP-8.2/.github/CODEOWNERS
[2] https://github.com/php/php-src/blob/master/.github/CODEOWNERSCheers,
Christoph
From what I can see, the difference is mostly me....
I'm in favor of syncing.
Regards,
Saki
Hi all!
I have noticed that the CODEOWNERS of php-src are apparently not synced
for all active branches[1][2]. However, it seems to me that this
doesn't make sense; if someone provides a PR against some lower branch,
the most recent CODEOWNERS may not be notified, and modifications might
be send to possibly inactive maintainers. This is particularly
confusing if the base branch of an PR will be changed; all over sudden
new reviews may be requested.So I suggest to sync CODEOWNERS for all active branches (and maybe even
security branches).What do you think?
[1] https://github.com/php/php-src/blob/PHP-8.2/.github/CODEOWNERS
[2] https://github.com/php/php-src/blob/master/.github/CODEOWNERSCheers,
Christoph
Hi
I believe they should be synced indeed, although we should be careful
with extensions that are dropped from PHP (e.g. imap). Those entries
should remain only in 8.2 and 8.3 but not in master.
Kind regards
Niels
Hi Niels!
I believe they should be synced indeed, although we should be careful
with extensions that are dropped from PHP (e.g. imap). Those entries
should remain only in 8.2 and 8.3 but not in master.
Oh, right! My idea was to handle this like bug fixes: target the lowest
branch which is relevant, and merge upwards as applicable (possibly do
empty merges up to master).
For those that may not have noticed yet: Peter Kokot provided PR #15017.
Maybe the details can be discussed there.
Cheers,
Christoph
Hi Christoph
So I suggest to sync CODEOWNERS for all active branches (and maybe even
security branches).What do you think?
I think back when it was introduced it wasn't clear (at least to me)
that PRs would request reviews based on the CODEOWNERS file of the
target branch, rather than the default branch. Given that's how it
works, syncing it in all active branches makes sense to me.
Ilija
Hi Christoph
On Thu, Jul 18, 2024 at 2:09 PM Christoph M. Becker cmbecker69@gmx.de
wrote:So I suggest to sync CODEOWNERS for all active branches (and maybe even
security branches).What do you think?
I think back when it was introduced it wasn't clear (at least to me)
that PRs would request reviews based on the CODEOWNERS file of the
target branch, rather than the default branch. Given that's how it
works, syncing it in all active branches makes sense to me.Ilija
I think that nothing will help solve this issue but to write also some
comment in that file on top to send updates to the earliest supported
branch, and not the master branch only.
We were resolving this in the past not so long ago:
https://github.com/php/php-src/pull/13591