Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119629
Message-ID: <cb1a463e-28f3-ba1a-739b-4ee40a73e5eb@gmail.com>
Date: Wed, 1 Mar 2023 09:34:10 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
To: Dmitry Stogov <dmitrystogov@gmail.com>, Max Kellermann <max+php@blarg.de>
Cc: PHP Internals <internals@lists.php.net>
References: <CAPVMwwvRyE7+XUHoQ=p=HBKdFrc7FaCAB52hyNzr79hLdhcZqQ@mail.gmail.com>
 <Y/5sY/xNqDGoKSW1@swift.blarg.de>
 <CAPVMwwsVLH8O5+KAN7khVowPmDULk+LdDGKTD9E16=qHAM-tAg@mail.gmail.com>
 <Y/6BThHaWBfUyWtd@swift.blarg.de> <Y/7knFJqhsF+Vu+v@swift.blarg.de>
 <CAPVMwwtdTHEXNeoigA6KVT=htDZ74Ta2=aRT09iRBBd9oNYUog@mail.gmail.com>
Content-Language: en-US
In-Reply-To: <CAPVMwwtdTHEXNeoigA6KVT=htDZ74Ta2=aRT09iRBBd9oNYUog@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] PHP code refactoring (was: include cleanup)
From: dossche.niels@gmail.com (Niels Dossche)

On 3/1/23 08:42, Dmitry Stogov wrote:
> On Wed, Mar 1, 2023 at 8:37 AM Max Kellermann <max+php@blarg.de> wrote:
> 
>> On 2023/02/28 23:33, Max Kellermann <max+php@blarg.de> wrote:
>>>> Include cleanups RFC was rejected.
>>>> No refactoring RFC was presented.
>>>> A lot of changes that affect all core contributors are committed into
>>>> master.
>>>
>>> Do you mean to imply that code changes that do not implement RFC or
>>> fix a bug should always be rejected?
>>
>> CONTRIBUTING.md says:
>>
>>  "PHP welcomes pull requests to add tests, fix bugs and to implement
>>  RFCs."
>>
>> Indeed it appears Dmitry is right - code refactoring is generally NOT
>> allowed (unless there is an explicit RFC vote, and I havn't seen one).
>>
>> This implies that all those commits (and hundreds of others):
>>
>>>
>> https://github.com/php/php-src/commit/4177257178d6a1a44f0aa6d6b23d02b91e0a58d3
>>>
>> https://github.com/php/php-src/commit/9108a32bfe881c3b1e2f3b2949b0e9fe1b9c6dda
>>>
>> https://github.com/php/php-src/commit/07fe46fb5db9d6f34e72f513ae053fc8c9ad67a
>>>
>> https://github.com/php/php-src/commit/900472536775b71d5d72a0d66eaa46ae7c7d7ad9
>>>
>> https://github.com/php/php-src/commit/f0cfebc2b867a6a96a88c4526cf9f3b4cd01f04b
>>>
>> https://github.com/php/php-src/commit/f079aa2e242b251c6297bddf5365c33c126b7dcc
>>>
>> https://github.com/php/php-src/commit/b14dd85dca3b67a5462f5ed9b6aa0dc22beb615c
>>
>> ... should not have been merged!
>>
> 
> At least they should be reviewed once again.
> I already have questions for the
> https://github.com/php/php-src/commit/9108a32bfe881c3b1e2f3b2949b0e9fe1b9c6dda
> 

Hi Dmitry

I made that commit.
Please ask me the questions you have and I'll be happy to answer them.

Thanks
Niels

> 
>> Where do we go from here?  Really revert EVERYTHING?
>>
> 
> I'm not too paranoid about small changes and changes in extensions code.
> I didn't object when you proposed small refactoring steps. (e.g. adding
> "const" and "static" or small include clenups).
> 
> 
>> (Reverting just my code refactoring changes but nobody else's would
>> make no sense at all.  Don't make this personal.)
>>
> 
> This is not personal. I saw you are smart and may find and fix not trivial
> bugs. I assume all contributors have good intentions.
> The problem with your commits, that you started rewriting EVERYTHING (the
> core parts of php) without a deep knowledge and without agreement with
> mainteners.
> They already started to break things (See Nikita's notes about RC
> debugger).
> Formally they were committed after a declined RFC.
> 
> I already proposed a way that might work.
> 
> Thanks. Dmitry.
> 
> 
> 
>>
>> Max
>>
>