Hi Max, Thanks for describing the situation.
We do not often vote on implementation changes that don't affect PHP language itself. At first, I even considered these changes as "I don't care". However, they are turned up into huge patches that make troubles for PHP maintenance. As we didn't come to an agreement, the best decision should be done by the PHP community. In the current state of the implementation I'm personally against this, but I might change my opinion if this is targeted to PHP-9 and the implementation is done in a some better way. Thanks. Dmitry. On Mon, Jan 16, 2023 at 3:03 PM Max Kellermann <max+...@blarg.de> wrote: > Hi, > > in the past weeks, I submitted four PRs for cleaning up the #includes > in the PHP code base: > > https://github.com/php/php-src/pull/10216 > https://github.com/php/php-src/pull/10220 > https://github.com/php/php-src/pull/10279 > https://github.com/php/php-src/pull/10300 > > I saw that the existing #include directives were inconsistent, > incomplete and bloated; things just worked by chance, not by design, > because there were a few headers which just included everything. I > wanted to help clean up this mess that had accumulated over two > decades. > > All PRs were welcomed by different reviewers and were merged; there > was just one minor criticism by Dmitry Stogov who thought the code > comments explaining many non-obvious #includes should be removed: > > https://github.com/php/php-src/pull/10216#issuecomment-1375140255 > > > I don't think we should include the comments like // for > > BEGIN_EXTERN_C (and similar). The are good for review only. I'm > > indifferent to these changes and don't object. > > Yesterday, when a DTrace-specific regression was reported > (https://github.com/php/php-src/pull/10220#issuecomment-1383035139), > after which Dmitry asked to revert the whole PR: > > https://github.com/php/php-src/pull/10220#issuecomment-1383658247 > > Instead, I submitted a trivial fix for the regression > (https://github.com/php/php-src/pull/10334), which was rejected by > Dmitry > (https://github.com/php/php-src/pull/10220#issuecomment-1383706602) > but confirmed by the original reporter > (https://github.com/php/php-src/pull/10220#issuecomment-1383802334). > > In spite of that, Dmitry demanded to revert all of my #include > cleanups > (https://github.com/php/php-src/pull/10220#issuecomment-1383739816): > > > I'm asking to revert all these include cleanup commits! This is > > just a useless permutation. e.g. 68ada76 adds typedef struct _* that > > we didn't need before. How is this clearly? > > ... which so far only Derick Rethans agreed to: > > https://github.com/php/php-src/pull/10220#issuecomment-1383784480 > > > FWIW, I agree with Dmitry here, and these should all be reverted. It > > adds nothing but clutter. > > Christoph M. Becker performed the revert and suggested doing an RFC > (https://github.com/php/php-src/pull/10220#issuecomment-1383789100) > and vote on it. > > So this is a first draft of my proposal which I'd like to discuss with > you: > > https://github.com/php/php-src/pull/10338 > > Max > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
