Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119620
Date: Tue, 28 Feb 2023 23:33:50 +0100
To: Dmitry Stogov <dmitrystogov@gmail.com>
Cc: PHP Internals <internals@lists.php.net>
Message-ID: <Y/6BThHaWBfUyWtd@swift.blarg.de>
References: <CAPVMwwvRyE7+XUHoQ=p=HBKdFrc7FaCAB52hyNzr79hLdhcZqQ@mail.gmail.com>
 <Y/5sY/xNqDGoKSW1@swift.blarg.de>
 <CAPVMwwsVLH8O5+KAN7khVowPmDULk+LdDGKTD9E16=qHAM-tAg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAPVMwwsVLH8O5+KAN7khVowPmDULk+LdDGKTD9E16=qHAM-tAg@mail.gmail.com>
Subject: Re: [PHP-DEV] PHP code refactoring (was: include cleanup)
From: max+php@blarg.de (Max Kellermann)

On 2023/02/28 23:08, Dmitry Stogov <dmitrystogov@gmail.com> wrote:
> > Which community rule was violated by whom?
> >
> 
> Merging the things that were rejected. You may name this differently but
> this is still code refactoring.

That sidesteps my question, and answers something else.

> In your opinion, what exactly does the outcome of the vote mean?  Does
> > it mean that include cleanups are now forbidden (despite being wanted
> > by the majority of voters)?  Or does it mean that nothing changes
> > because no new rule has taken effect?  (I asked that already, but
> > nobody replied.)
> >
> 
> 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?

If that is true, I'll point you to a huge bunch of refactoring commits
that must be rejected until an RFC is formally accepted by a
supermajority.

Just to name a few very recent ones:

 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

Do you believe these should be reverted as well?

> > What do you mean by "merged silently"?
> >
> 
> I don't review every PR. Only when I asked. I'm mainly busy with new
> development and bug fixes.

That doesn't answer my question.  You said my work as merged
"silently", and I'd like to know what you mean by that.

> > Though, by contrast, you merge most of your own PRs shortly after you
> > create them without any code review.  Quite often, you even push
> > commits without any PR.  In my opinion, that's more "silent" than my
> > work, don't you agree?
> >
> 
> Currently, I merge into master only bug fixes.
> And yes, sometimes my fixes may introduce new bugs.
> My new development is done in a separate branch and will be presented in
> RFC when ready.

That, again, doesn't answer my question.  Do you mean that merging bug
fixes is allowed to be "silent"?  (Whatever "silent" means.)

> > Which specific commits do you wish to revert?  Is this about include
> > cleanups (none of which were merged) or about header splitting (which
> > a supermajority voted for) or about other kinds of code refactoring
> > (which was not voted on)?
> >
> 
> All code refactoring commits.

Okay, so you demand to revert all code refactoring commits - all of
them, not just the ones I authored?  Including the ones I linked
above?

> Can you imagine you commited something like this into Linux, JVM, GCC?

Yes.  That happens all the time in those projects.  I'm surprised by
your conservatism.

> PHP-6 was developed in a separate branch and we were able to stop it when
> understood that it is not good enough.
> PHPNG was developed in a separate branch and was presented and accepted
> only when we showed the benefits.

That's different.  These were major rewrites, and my work is small
incremental improvements, just formal changes, no actual code changes.

> A new JIT has been developed in a separate branch for more than a year...

The new JIT, the one that you used as a reason to reject all
changes to the current JIT, because changing the current JIT would
cause merge conflicts for your new JIT?  IMO that demonstrates what's
wrong with that approach.

Max