? Pending

User tests: Successful: Unsuccessful:

avatar PhilETaylor
PhilETaylor
26 Apr 2021

[4] Non-Controversial CodeStyle Cleanup for /components

Code review

avatar PhilETaylor PhilETaylor - open - 26 Apr 2021
avatar PhilETaylor PhilETaylor - change - 26 Apr 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 26 Apr 2021
Category Front End com_banners com_config com_contact com_content
avatar PhilETaylor PhilETaylor - change - 26 Apr 2021
Labels Added: ?
avatar joomdonation
joomdonation - comment - 27 Apr 2021

@PhilETaylor Do you think PR like this will cause merge conflicts with:

  • Existing PRs
  • Or when @wilsonge merge the changes from staging and 3.10 to 4.0-dev

For these reasons, maybe we should do it a bit later ?

avatar PhilETaylor
PhilETaylor - comment - 27 Apr 2021

For these reasons, maybe we should do it a bit later ?

That was @wilsonge excuse was almost 5 years ago when I tried to improve the quality of the code by importing classes.

Im sorry but I dont believe it should be left. Merge conflicts are easy - very easy - to resolve, and this PR fully complies with all the Joomla Coding Standards and best practice and there is no reason why it should not be tested and merged.

Furthermore there is a performance optimisation to be gained by using namespaced, opcache compiled, functions and so delaying it will make Joomla 4 less performant than it could/should be (by a tiny tiny amount)

Ultimately its not my decision, however if this PR and the others are rejected, it says a lot about the quality of the code and the project as a whole.

avatar rdeutz
rdeutz - comment - 27 Apr 2021

I don't think it is the right moment for massive code (style) changes, we should concentrate on the important things to get 4.0.0 ready. Changing a lot of files will make upstream merges more complicated as they are already. I don't will mege any of the code style PRs. We can revisit this after we have released 4.0.0

avatar PhilETaylor
PhilETaylor - comment - 27 Apr 2021

Well that's your opinion, and as I said - speaks volumes about the project.

These changes are bringing the code in line with the Joomla Coding standards. The code should already be complying with those standards.

Ive been hearing the same excuses for over 5 years now. There will never be a right time.

avatar PhilETaylor
PhilETaylor - comment - 27 Apr 2021

We can revisit this after we have released 4.0.0

That will never happen.

avatar PhilETaylor
PhilETaylor - comment - 27 Apr 2021

I don't think it is the right moment for massive code (style) changes

So you are happy that Joomla code doesn't comply with Joomla Coding Standards... Got it.

avatar PhilETaylor
PhilETaylor - comment - 27 Apr 2021

Closing as no one has the balls to merge this.

avatar PhilETaylor PhilETaylor - change - 27 Apr 2021
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2021-04-27 14:02:03
Closed_By PhilETaylor
avatar PhilETaylor PhilETaylor - close - 27 Apr 2021
avatar PhilETaylor
PhilETaylor - comment - 27 Apr 2021

we should concentrate on the important things to get 4.0.0 ready

Like the 1980s FTP Layer??? Yeah right...

avatar rdeutz
rdeutz - comment - 27 Apr 2021

So you are happy that Joomla code doesn't comply with Joomla Coding Standards... Got it.

Would be nicer, if you don't put words into my mouth.

avatar PhilETaylor
PhilETaylor - comment - 27 Apr 2021

It would be nicer if maintainers maintained, and merged, but here we are...

avatar PhilETaylor
PhilETaylor - comment - 27 Apr 2021

Im struggling to think of another project that blocks progress "because it would make merging harder"...

Merging = Selecting from the left or right and clicking save, sometimes making a few text changes

Merging is not difficult !

Add a Comment

Login with GitHub to post a comment