Hi @internals,
I have to say that we came to a serious conflict.
Recently we voted for inluce cleanup RFC
https://wiki.php.net/rfc/include_cleanup and it was declined.
Despite that a series of code refactoring commits from Max were silently
merged into the master.
As this is a violation of the community rules and we should do something.
In my opinion, this should be reverted (I would even think about rebasing
to not pollute the git history).
Personally, I don't like this refactoring, because it is mainly about
coding preferences and habits.
Anyway, I see that some people like this. Maybe this may attract new
developers.
OK. Let's do this, but do this in a managed way. The massive uncontrolled
changes is the main problem in the current situation.
Let's define the goal(s), rules, make a plan, summarize this in a new RFC,
accept it.
Then most of the work should be done in a separate branch and merged into
the master all together after a final review.
I (and other authority contributors) wouldn't be able to object against the
terms accepted in RFC.
So a good RFC should be a half of success...
It would be great if PHP Foundation could assign some experienced
developer(s) to be part of this work.
Thanks. Dmitry.
Which commit?
Hi @internals,
I have to say that we came to a serious conflict.
Recently we voted for inluce cleanup RFC
https://wiki.php.net/rfc/include_cleanup and it was declined.
Despite that a series of code refactoring commits from Max were silently
merged into the master.
As this is a violation of the community rules and we should do something.
In my opinion, this should be reverted (I would even think about rebasing
to not pollute the git history).Personally, I don't like this refactoring, because it is mainly about
coding preferences and habits.
Anyway, I see that some people like this. Maybe this may attract new
developers.OK. Let's do this, but do this in a managed way. The massive uncontrolled
changes is the main problem in the current situation.
Let's define the goal(s), rules, make a plan, summarize this in a new RFC,
accept it.
Then most of the work should be done in a separate branch and merged into
the master all together after a final review.
I (and other authority contributors) wouldn't be able to object against the
terms accepted in RFC.
So a good RFC should be a half of success...
It would be great if PHP Foundation could assign some experienced
developer(s) to be part of this work.Thanks. Dmitry.
Which commit?
just some of them:
https://github.com/php/php-src/commit/0270a1e54c0285fa3c89ee2b0120073ef57ab5fa
https://github.com/php/php-src/commit/b98f18e7c3838cf587a1b6d0f033b89e9909c79d
https://github.com/php/php-src/commit/42577c6b6b7577c57c161ee4a74cb193382bf1e0
https://github.com/php/php-src/commit/c7637ed1c03f556c6fb65884cfc5bfea4920b1c7
https://github.com/php/php-src/commit/371ae12d890f1887f79b7e2a32f808b4595e5f60
Thanks. Dmitry.
On Tue, Feb 28, 2023, 3:17 PM Dmitry Stogov dmitrystogov@gmail.com
wrote:Hi @internals,
I have to say that we came to a serious conflict.
Recently we voted for inluce cleanup RFC
https://wiki.php.net/rfc/include_cleanup and it was declined.
Despite that a series of code refactoring commits from Max were silently
merged into the master.
As this is a violation of the community rules and we should do something.
In my opinion, this should be reverted (I would even think about rebasing
to not pollute the git history).Personally, I don't like this refactoring, because it is mainly about
coding preferences and habits.
Anyway, I see that some people like this. Maybe this may attract new
developers.OK. Let's do this, but do this in a managed way. The massive uncontrolled
changes is the main problem in the current situation.
Let's define the goal(s), rules, make a plan, summarize this in a new RFC,
accept it.
Then most of the work should be done in a separate branch and merged into
the master all together after a final review.
I (and other authority contributors) wouldn't be able to object against
the
terms accepted in RFC.
So a good RFC should be a half of success...
It would be great if PHP Foundation could assign some experienced
developer(s) to be part of this work.Thanks. Dmitry.
https://github.com/php/php-src/commit/0270a1e54c0285fa3c89ee2b0120073ef57ab5fa
This kind of change was favored by a supermajority.
You argue that this supermajority vote is irrelevant, and formally it
indeed is, but pondering about formalities is kind of ignorant against
the now well-known community opinion.
https://github.com/php/php-src/commit/b98f18e7c3838cf587a1b6d0f033b89e9909c79d
No vote was made on this, therefore this doesn't violate any community
rules, does it?
If you think this should be reverted, explain why.
https://github.com/php/php-src/commit/42577c6b6b7577c57c161ee4a74cb193382bf1e0
Favored by supermajority, see above.
https://github.com/php/php-src/commit/c7637ed1c03f556c6fb65884cfc5bfea4920b1c7
No vote, no rule violation, see above.
https://github.com/php/php-src/commit/371ae12d890f1887f79b7e2a32f808b4595e5f60
As you see in the commit message, this implements an (unwritten) rule
cited by Nikita Popov (which is now written as of
https://github.com/php/php-src/pull/10630). I personally don't agree
with this rule (there's a thread on this mailing list about it), and I
would favor reverting this commit - I only submitted this trying to
help with implementing a rule even though I don't agree with it.
If this gets reverted, then https://github.com/php/php-src/pull/10630
should be reverted as well. Again, not my opinion, I'm just trying to
help implement somebody else's opinion.
Max
https://github.com/php/php-src/commit/0270a1e54c0285fa3c89ee2b0120073ef57ab5fa
This kind of change was favored by a supermajority.
You argue that this supermajority vote is irrelevant, and formally it
indeed is, but pondering about formalities is kind of ignorant against
the now well-known community opinion.https://github.com/php/php-src/commit/b98f18e7c3838cf587a1b6d0f033b89e9909c79d
No vote was made on this, therefore this doesn't violate any community
rules, does it?
Please reread https://wiki.php.net/RFC/voting#voting
RFC is accepted by a supermajority of the primary vote.
The secondary votes may be used to make decisions about implementation
details.
Thanks. Dmitry.
If you think this should be reverted, explain why.
https://github.com/php/php-src/commit/42577c6b6b7577c57c161ee4a74cb193382bf1e0
Favored by supermajority, see above.
https://github.com/php/php-src/commit/c7637ed1c03f556c6fb65884cfc5bfea4920b1c7
No vote, no rule violation, see above.
https://github.com/php/php-src/commit/371ae12d890f1887f79b7e2a32f808b4595e5f60
As you see in the commit message, this implements an (unwritten) rule
cited by Nikita Popov (which is now written as of
https://github.com/php/php-src/pull/10630). I personally don't agree
with this rule (there's a thread on this mailing list about it), and I
would favor reverting this commit - I only submitted this trying to
help with implementing a rule even though I don't agree with it.If this gets reverted, then https://github.com/php/php-src/pull/10630
should be reverted as well. Again, not my opinion, I'm just trying to
help implement somebody else's opinion.Max
https://github.com/php/php-src/commit/b98f18e7c3838cf587a1b6d0f033b89e9909c79d
No vote was made on this, therefore this doesn't violate any community
rules, does it?Please reread https://wiki.php.net/RFC/voting#voting
RFC is accepted by a supermajority of the primary vote.
The secondary votes may be used to make decisions about implementation
details.
That doesn't answer my question. Does it violate any community rules
or not?
Max
Hey Max and Dmitry,
Am 28.02.2023 um 23:34 schrieb Dmitry Stogov dmitrystogov@gmail.com:
https://github.com/php/php-src/commit/0270a1e54c0285fa3c89ee2b0120073ef57ab5fa
This kind of change was favored by a supermajority.
You argue that this supermajority vote is irrelevant, and formally it
indeed is, but pondering about formalities is kind of ignorant against
the now well-known community opinion.https://github.com/php/php-src/commit/b98f18e7c3838cf587a1b6d0f033b89e9909c79d
No vote was made on this, therefore this doesn't violate any community
rules, does it?Please reread https://wiki.php.net/RFC/voting#voting
RFC is accepted by a supermajority of the primary vote.
The secondary votes may be used to make decisions about implementation
details.Thanks. Dmitry.
In this case, while the primary concern of the RFC was rejected, I think it's pretty clear, that there was a supermajority for something specific.
I do agree, that the topic, given hat it conflicts with the rules, a mail to internals on that topic should have been sent.
I did vote no on that topic, however I still think there was some mandate by the community that this is wanted.
I would thus not make too much of a fuss on these grounds about the splitting.
If you think this should be reverted, explain why.
https://github.com/php/php-src/commit/42577c6b6b7577c57c161ee4a74cb193382bf1e0
Favored by supermajority, see above.
https://github.com/php/php-src/commit/c7637ed1c03f556c6fb65884cfc5bfea4920b1c7
No vote, no rule violation, see above.
This is no violation and I think Dmitrys tone was a bit too accusing for that. But I also strongly disagree with that change.
Let's revert it. Quickly seeing which number is which type is quite valuable for debugging.
By now I know most numbers by heart, but most people won't.
https://github.com/php/php-src/commit/371ae12d890f1887f79b7e2a32f808b4595e5f60
As you see in the commit message, this implements an (unwritten) rule
cited by Nikita Popov (which is now written as of
https://github.com/php/php-src/pull/10630). I personally don't agree
with this rule (there's a thread on this mailing list about it), and I
would favor reverting this commit - I only submitted this trying to
help with implementing a rule even though I don't agree with it.If this gets reverted, then https://github.com/php/php-src/pull/10630
should be reverted as well. Again, not my opinion, I'm just trying to
help implement somebody else's opinion.
No, it should not. It makes a lot of sense.
It's however not a hard rule, but a guideline.
Applying these changes to ZEND_API though always shall be evaluated with due care.
In this specific instance, a fairly new API I think nobody uses yet, it's sort of acceptable.
I wouldn't mind reverting it though
Max
Max, while it's nice to see a couple cleanups in the PHP codebase, I would like to ask you to be a bit less quick at moving things around.
There's value in having a codebase evaluate slowly. Obviously not at the cost of progress, new features etc..
Keep in mind, that at least extension authors will need to work with the php-src codebase of version 8.2 until probably 7 years in the future (basically until PHP is EOL on all Linux distributions). They will appreciate not looking at a vastly different codebases.
Finally, these sorts of code moves are also making a git blame harder to track back.
Thanks,
Bob
Am 01.03.2023 um 01:13 schrieb Bob Weinand bobwei9@hotmail.com:
In this case, while the primary concern of the RFC was rejected, I think it's pretty clear, that there was a supermajority for something specific.
I didn't vote on this RFC but I have to disagree with you and Max here: My understanding of such a voting process is that the secondary votes are dependent on the acceptance of the first one.
This means I would interpret it as 'if and only if "Should #include directives be cleaned up?" Is accepted then I'm voting yes for "Is it allowed to split a large header to reduce dependencies"'.
I can't know whether the people voting here meant it that way or not, I'm just saying that taking the second vote out of context seems problematic to me. Reminds me a bit of looking at the numbers of an A/B test when the precondition "is the result significant" is false.
Regards,
- Chris
I'm a bit out of the loop on the higher level discussion, but as I got named dropped here, a quick note...
https://github.com/php/php-src/commit/0270a1e54c0285fa3c89ee2b0120073ef57ab5fa
This kind of change was favored by a supermajority.
You argue that this supermajority vote is irrelevant, and formally it
indeed is, but pondering about formalities is kind of ignorant against
the now well-known community opinion.https://github.com/php/php-src/commit/b98f18e7c3838cf587a1b6d0f033b89e9909c79d
No vote was made on this, therefore this doesn't violate any community
rules, does it?If you think this should be reverted, explain why.
https://github.com/php/php-src/commit/42577c6b6b7577c57c161ee4a74cb193382bf1e0
Favored by supermajority, see above.
https://github.com/php/php-src/commit/c7637ed1c03f556c6fb65884cfc5bfea4920b1c7
No vote, no rule violation, see above.
This commit broke a valuable debugging reference: I used to often check these defines to determine what a type code means. If you get type 18, good luck figuring out what that means now.
https://github.com/php/php-src/commit/371ae12d890f1887f79b7e2a32f808b4595e5f60
As you see in the commit message, this implements an (unwritten) rule
cited by Nikita Popov (which is now written as of
https://github.com/php/php-src/pull/10630). I personally don't agree
with this rule (there's a thread on this mailing list about it), and I
would favor reverting this commit - I only submitted this trying to
help with implementing a rule even though I don't agree with it.
This is a pretty cherry-picked statement. In that mailing list thread I explicitly pointed out that while this is the rule (and if you introduce new code, that would be the convention to follow), that does not necessarily mean that it's a good idea to fix existing cases that don't follow it. I also pointed out that for public APIs, this is a very insidious API break, because it doesn't make using code fail to compile: It just silently inverts the result from it's previous meaning. It looks like those parts of my mail just got ignored.
Regards,
Nikita
If this gets reverted, then https://github.com/php/php-src/pull/10630
should be reverted as well. Again, not my opinion, I'm just trying to
help implement somebody else's opinion.Max
--
To unsubscribe, visit: https://www.php.net/unsub.php
Recently we voted for inluce cleanup RFC
https://wiki.php.net/rfc/include_cleanup and it was declined.
Despite that a series of code refactoring commits from Max were silently
merged into the master.
As this is a violation of the community rules and we should do something.
I don't get any of that. Please be more specific.
Which community rule was violated by whom?
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.)
What do you mean by "merged silently"? All of my PRs were submitted
in the public, and everybody had a chance to comment on these (which
you did, for example, in https://github.com/php/php-src/pull/10641).
It usually takes a week or so before they are merged. If you feel
that is not enough time, why not post a RFC and vote on a mandatory
minimum review duration for all merges/pushes?
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?
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)?
Then most of the work should be done in a separate branch and merged
into the master all together after a final review.
The problem with merge conflicts was your major complaint about branch
merging (and that was only about merging bug fixes, not about merging
two volatile branches together) - and now you suggest something that
will certainly lead to merge conflicts because your suggestion
postpones the big merge for as long as possible. That will inflict
major pain to PHP maintainers (though not to outside contributors like
me, so why would I care).
Another big problem: if it's uncertain that some changes ever get
merged, because the "final review" may reject it after months or years
of work, nobody will ever want to clean up the PHP code again, because
it's very likely that you'll veto it again, and then all the time
would be wasted. Not a great prospect. If you want to scare away
contributors, that's how to do it.
My opinion is: if a patch improves a piece of code, it should be
merged. Of course, whether something is an improvement is debatable;
but postponing that decision is pointless and harmful.
Max
Am 28.02.2023 um 22:04 schrieb Max Kellermann max+php@blarg.de:
Recently we voted for inluce cleanup RFC
https://wiki.php.net/rfc/include_cleanup and it was declined.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)?
You keep saying that people voted yes on the secondary vote (clarifications on how to clean up).
Just to clarify: This is irrelevant as the primary vote was declined.
My opinion is: if a patch improves a piece of code, it should be
merged. Of course, whether something is an improvement is debatable;
but postponing that decision is pointless and harmful.
As far as I can see there seems to be a disagreement whether it is an improvement, which to me sounds like there must be more discussion. Which means the decision has to be postponed, no?
I second Dmitry's request to do things in a managed way.
What exactly caused the disruption I'm not sure and it is not about blaming someone but finding a workable solution.
Regards,
- Chris
Recently we voted for inluce cleanup RFC
https://wiki.php.net/rfc/include_cleanup and it was declined.
Despite that a series of code refactoring commits from Max were silently
merged into the master.
As this is a violation of the community rules and we should do something.I don't get any of that. Please be more specific.
Which community rule was violated by whom?
Merging the things that were rejected. You may name this differently but
this is still code refactoring.
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.
If you think this is questionable, we may ask for help from some arbitr.
What do you mean by "merged silently"? All of my PRs were submitted
in the public, and everybody had a chance to comment on these (which
you did, for example, in https://github.com/php/php-src/pull/10641).
It usually takes a week or so before they are merged. If you feel
that is not enough time, why not post a RFC and vote on a mandatory
minimum review duration for all merges/pushes?
I don't review every PR. Only when I asked. I'm mainly busy with new
development and bug fixes.
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.
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.
Can you imagine you commited something like this into Linux, JVM, GCC?
Then most of the work should be done in a separate branch and merged
into the master all together after a final review.The problem with merge conflicts was your major complaint about branch
merging (and that was only about merging bug fixes, not about merging
two volatile branches together) - and now you suggest something that
will certainly lead to merge conflicts because your suggestion
postpones the big merge for as long as possible. That will inflict
major pain to PHP maintainers (though not to outside contributors like
me, so why would I care).
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.
A new JIT has been developed in a separate branch for more than a year...
Another big problem: if it's uncertain that some changes ever get
merged, because the "final review" may reject it after months or years
of work, nobody will ever want to clean up the PHP code again, because
it's very likely that you'll veto it again, and then all the time
would be wasted. Not a great prospect. If you want to scare away
contributors, that's how to do it.
As I said, I won't object to the terms of accepted RFC.
I already made much more noise than I liked.
My opinion is: if a patch improves a piece of code, it should be
merged. Of course, whether something is an improvement is debatable;
but postponing that decision is pointless and harmful.
Changing code back and force without a plan is also harmful.
I proposed to you a way to do what you like in agreement with other
contributors.
Doing this without agreement won't work.
Thanks. Dmitry.
Max
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
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!
Where do we go from here? Really revert EVERYTHING?
(Reverting just my code refactoring changes but nobody else's would
make no sense at all. Don't make this personal.)
Max
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
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
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
On Wed, Mar 1, 2023 at 11:34 AM Niels Dossche dossche.niels@gmail.com
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 thehttps://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.
See the comment at the commit page on github and let's continue the related
discussion there.
I think github sends notification emails to commit authors.
You probably missed it.
Thanks. Dmitry.
Thanks
NielsWhere 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
Recently we voted for inluce cleanup RFC
https://wiki.php.net/rfc/include_cleanup and it was declined.
Despite that a series of code refactoring commits from Max were silently
merged into the master.
As this is a violation of the community rules and we should do something.I don't get any of that. Please be more specific.
Which community rule was violated by whom?
The one where you shouldn't have tried your hardest to antagonise as many people as possible.
cheers
Derick