Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119318
Date: Wed, 18 Jan 2023 17:07:52 +0100
To: internals@lists.php.net
Message-ID: <Y8gZWLQd1SvL6a3n@swift.blarg.de>
References: <Y8U9B4AVP/R7vazI@swift.blarg.de>
 <CAGBsUrdQAXE2qk_UDQY9v6pYpWH+=bX6S3ou+sttQamtDC1F9A@mail.gmail.com>
 <Y8VI5yanzli8D4Cm@swift.blarg.de>
 <alpine.DEB.2.23.453.2301181536160.23089@singlemalt.home.derickrethans.nl>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.23.453.2301181536160.23089@singlemalt.home.derickrethans.nl>
Subject: Re: [PHP-DEV] RFC: rules for #include directives
From: max+php@blarg.de (Max Kellermann)

On 2023/01/18 16:41, Derick Rethans <derick@php.net> wrote:
> Including a "random" zend header also sounds like a great benefit. I 
> shouldn't need to care as an extension author which header enables which 
> internal function(s) for usage.

So, in your opinion, choosing an arbitrary of the 95 Zend/zend*.h
headers should make *all* functions available?

I don't agree with that.  Having a documented header such as
"zend_all.h" (or just the already-existing "php.h") which really makes
everything available is certainly a good idea.  But making all 95
headers behave that way doesn't sound like a good design.

> This is a very naive view of the world.

You make this personal.  Let's not.

> When looking at the commit history, I saw more than a dozen commits 
> doing a small change. That is clutter.

 https://github.com/php/php-src/blob/master/CONTRIBUTING.md

 "Do not commit multiple files and dump all messages in one commit. If
 you modified several unrelated files, commit each group separately
 and provide a nice commit message for each one."

Each of my commits is self-contained and cleans up one internal
library.  Exactly the way the PHP commit rules ask me to do.

I even took extra care so every commit is buildable, even though I had
to reorder them carefully to make that possible.  You may or may not
value my effort, but it is in line with PHP's rules.

> Adding forwards declarations for zend_* structs, is clutter.

That is a reasonable opinion, but not mine.

> Adding comments that go out of date as to why a header is included is 
> also clutter.

This argument puzzles me most.  I've never heard anybody criticize
some piece of code for having too MANY code comments, too MUCH
explanation.

Every piece of documentation/explanation eventually goes out of date,
but that's not a good reason to never write any.

> > Dmitry Stogov said it's "just a useless permutation", but I can't 
> > understand this argument either.
> 
> It creates code-churn, which makes merging up fixes from older branches 
> harder. 

I've been rebasing my include cleanup patches for several weeks and
have yet to see the first conflict.  That's because bug fixes merged
from the stable branches rarely ever affect #include directives.  And
even if there is some day a merge conflict, fixing it is trivial.

Max