Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119628
MIME-Version: 1.0
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>
In-Reply-To: <Y/7knFJqhsF+Vu+v@swift.blarg.de>
Date: Wed, 1 Mar 2023 10:42:00 +0300
Message-ID: <CAPVMwwtdTHEXNeoigA6KVT=htDZ74Ta2=aRT09iRBBd9oNYUog@mail.gmail.com>
To: Max Kellermann <max+php@blarg.de>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000a27ab705f5d1d90a"
Subject: Re: [PHP-DEV] PHP code refactoring (was: include cleanup)
From: dmitrystogov@gmail.com (Dmitry Stogov)

--000000000000a27ab705f5d1d90a
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 1, 2023 at 8:37=E2=80=AFAM Max Kellermann <max+php@blarg.de> wr=
ote:

> 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/4177257178d6a1a44f0aa6d6b23d02b91e0=
a58d3
> >
> https://github.com/php/php-src/commit/9108a32bfe881c3b1e2f3b2949b0e9fe1b9=
c6dda
> >
> https://github.com/php/php-src/commit/07fe46fb5db9d6f34e72f513ae053fc8c9a=
d67a
> >
> https://github.com/php/php-src/commit/900472536775b71d5d72a0d66eaa46ae7c7=
d7ad9
> >
> https://github.com/php/php-src/commit/f0cfebc2b867a6a96a88c4526cf9f3b4cd0=
1f04b
> >
> https://github.com/php/php-src/commit/f079aa2e242b251c6297bddf5365c33c126=
b7dcc
> >
> https://github.com/php/php-src/commit/b14dd85dca3b67a5462f5ed9b6aa0dc22be=
b615c
>
> ... 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/9108a32bfe881c3b1e2f3b2949b0e9fe1b9c6=
dda


> 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
>

--000000000000a27ab705f5d1d90a--