Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:119275 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 48000 invoked from network); 16 Jan 2023 12:49:08 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 16 Jan 2023 12:49:08 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 5D8091804D4 for ; Mon, 16 Jan 2023 04:49:07 -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=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,HTML_MESSAGE, RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.2 X-Spam-ASN: AS15169 209.85.128.0/17 X-Spam-Virus: No X-Envelope-From: Received: from mail-pg1-f173.google.com (mail-pg1-f173.google.com [209.85.215.173]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Mon, 16 Jan 2023 04:49:06 -0800 (PST) Received: by mail-pg1-f173.google.com with SMTP id 78so19583646pgb.8 for ; Mon, 16 Jan 2023 04:49:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=OBtup5nykHruizDSEghs2eSSmLr10YbUpXX7vBF6bM4=; b=XZ084WFrc35LZqHnycsKBnLiUnkY3GzG+C2KWWAD7yN53dPV/vII6YWm6QjJ4b/dXr mL0JL5KxebAevN5sPiGYMNAXX3x92SNSrN24PPsMLVPYY/YStZ1eZXwdUUfaz3M6/m5n FTog3MftzEE9iLjvWriMFWYClaXgpBYzrCsxCCzKyqPexG8HxopcwTxFBPom4Pa88isa Ln+MZRQlwrnF8PH/0oSCB4EzdXO8MEzbep4Z+yqk8kDYKNsm/RdyZ+bu+ehofaQEaW7G WAEgczw5CTLCvecuu63Ys3mtpcxJqe3PoY1XgeyyHue2oWw8sSP+ypz2yaxMGwHZ+t2D xwjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=OBtup5nykHruizDSEghs2eSSmLr10YbUpXX7vBF6bM4=; b=hOI19wProZ+jjGhoAwRNIyQij7E0qzBhxhyfgmjNGV70nl7ji0w5uZ+ryWe9hlT1NL 3+8MpzHFNPYWmaA2j6/iO6Hox2wgXbvI1hcBtj0ww//8YNZK6YqSs4pdyxtnXQ6YIypY aPm8aJR+VLb18fOKkE1/7wRyBuG168LknCKqbcI7fm+wKrywmIu+zTVkfw5wnpiam6c6 GUq8fosA45h9o7Ye96zBTXVTMBB6QOPQQ7TvYKNrzKRk4q3ypNVmTIGf2k5xexDQBPvz NOaTWBGd8FeJp4cs0Mkjy+Et3fZWejVUQKuViqt16EmHPFNEC9qoz47pZ0SC11CaKPpk IG8g== X-Gm-Message-State: AFqh2kpiYkpS9/ikO7ocKAX5MES8kP6C4kTpcrdAXg56WVLszDToT2C1 50XfMXIVnyBEZZlw9WAQsScxlo7fo6Zbrmg+4RaLmdq1Z+s= X-Google-Smtp-Source: AMrXdXuBgmL+6Huu5ZJtG79SEUTv8WoGB52bjPnpiPbjS50rZHGZ12/YdjflbUSsUsGKS0m9IJd5h5KmEP5Wvsb2phI= X-Received: by 2002:a62:1482:0:b0:586:4e66:eadd with SMTP id 124-20020a621482000000b005864e66eaddmr2793828pfu.71.1673873345672; Mon, 16 Jan 2023 04:49:05 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: Date: Mon, 16 Jan 2023 12:48:54 +0000 Message-ID: To: Max Kellermann Cc: internals@lists.php.net Content-Type: multipart/alternative; boundary="0000000000002afce305f2610285" Subject: Re: [PHP-DEV] RFC: rules for #include directives From: george.banyard@gmail.com ("G. P. B.") --0000000000002afce305f2610285 Content-Type: text/plain; charset="UTF-8" On Mon, 16 Jan 2023 at 12:03, Max Kellermann 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 > As the person who has signed off on the PRs and merged them, I am in favour of these changes and think the revert is a total over reaction. Before merging the first set, it was asked ahead of time if those changes were OK, and multiple people were in favour of these changes. The fact the DTrace build was broken inadvertently and only found out today is a failure of our CI Build matrix and myself for not knowing this option nor compiling with it. Not of the changes. Moreover, having those sorts of changes be RFCs seems counterproductive as the only people who care about this are actual core and extensions developers and this opens the gate for petty RFCs to resolve coding style disagreements. Finally, some of the wording used in those PR replies are far from civil and can push away new contributors to the language. Best regards, George P. Banyard --0000000000002afce305f2610285--