Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:119272 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 42367 invoked from network); 16 Jan 2023 12:03:22 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 16 Jan 2023 12:03:22 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 46E0C18037F for ; Mon, 16 Jan 2023 04:03:22 -0800 (PST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-0.5 required=5.0 tests=BAYES_05,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.2 X-Spam-ASN: AS24940 138.201.0.0/16 X-Spam-Virus: No X-Envelope-From: Received: from swift.blarg.de (swift.blarg.de [138.201.185.127]) by php-smtp4.php.net (Postfix) with ESMTP for ; Mon, 16 Jan 2023 04:03:21 -0800 (PST) Received: from swift.blarg.de (swift.blarg.de [IPv6:2a01:4f8:c17:52a8::2]) (Authenticated sender: max) by swift.blarg.de (Postfix) with ESMTPSA id C979A41116 for ; Mon, 16 Jan 2023 13:03:20 +0100 (CET) Date: Mon, 16 Jan 2023 13:03:19 +0100 To: internals@lists.php.net Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Subject: RFC: rules for #include directives From: max+php@blarg.de (Max Kellermann) 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