? Pending

User tests: Successful: Unsuccessful:

avatar laoneo
laoneo
21 Jan 2017

As PSR-0 is deprecated, this PR offers to register namespaces also for PSR-4. The plan is to remove support for PSR-0 in J4 probably.

Maintainer review.

Ping @wilsonge

avatar laoneo laoneo - open - 21 Jan 2017
avatar laoneo laoneo - change - 21 Jan 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 21 Jan 2017
Category Libraries
avatar mbabker
mbabker - comment - 21 Jan 2017

Why can't both be supported? True that PSR-0 is deprecated but that doesn't mean it is unusable. When I see a platform like Composer deprecate/drop PSR-0 support I might find this acceptable but for now I don't see any reason to do anything but add a deprecation notice and leave it be until the greater PHP ecosystem drops it in full.

Also, this change will 100% break the patch tester component because it does use the PSR-0 autoloader as I've namespaced the component classes. The only way to therefore create a 3.x/4.x compatible build is to ship my own Composer autoloader since there won't be a path that allows me to load namespaced code in JLoader compatible with both versions.

avatar wilsonge wilsonge - change - 21 Jan 2017
Labels Added: ?
avatar wilsonge
wilsonge - comment - 21 Jan 2017

You cannot use both PSR-4 and PSR-0 code at the same time. So are we going to be stuck using PSR-0 code for another 5 years until composer removes support?

avatar mbabker
mbabker - comment - 21 Jan 2017

You can use both. Look at our Framework packages as an example. Are you saying I cannot install joomla/application and joomla/session together because the former uses PSR-4 and the latter PSR-0?

avatar mbabker
mbabker - comment - 21 Jan 2017

What you cannot do is register a path that's designed for PSR-4 support to a PSR-0 autoloader or vice versa. But you can most assuredly have an autoloader for each variant.

avatar wilsonge
wilsonge - comment - 21 Jan 2017

No but you must register each namespace with the relevant autoloader. Are we really going to introduce a registerPSR4Namespace method to register those namespaces differently.

avatar mbabker
mbabker - comment - 21 Jan 2017

I would suggest for 4.0 that you support both. Otherwise you're making the 4.0 upgrade path more complicated for the sake of it.

avatar mbabker
mbabker - comment - 21 Jan 2017

Either way I will now update the patch tester to isolate itself from the core autoloader to prevent possible unneeded breakage either now or in the future.

avatar wilsonge
wilsonge - comment - 21 Jan 2017

img_5991

Fiiiiine

avatar laoneo laoneo - change - 21 Jan 2017
Labels Added: ?
Removed: ?
avatar laoneo laoneo - change - 21 Jan 2017
The description was changed
avatar laoneo laoneo - edited - 21 Jan 2017
avatar laoneo laoneo - change - 21 Jan 2017
The description was changed
avatar laoneo laoneo - edited - 21 Jan 2017
avatar laoneo
laoneo - comment - 21 Jan 2017

When this one will be accepted PR #13673 needs to be adjusted as well.

avatar zero-24
zero-24 - comment - 21 Jan 2017

Travis error:

FILE: /home/travis/build/joomla/joomla-cms/libraries/loader.php

FOUND 1 ERROR(S) AFFECTING 1 LINE(S)

391 | ERROR | Parameters must appear immediately after the comment

UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY

avatar wilsonge
wilsonge - comment - 22 Jan 2017

Merging on review, given this is never going to get tests and we need it to progress in the sprint

avatar wilsonge wilsonge - change - 22 Jan 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-01-22 11:34:10
Closed_By wilsonge
avatar wilsonge wilsonge - close - 22 Jan 2017
avatar wilsonge wilsonge - merge - 22 Jan 2017
avatar zero-24
zero-24 - comment - 22 Jan 2017

I have just fixed some smal CS Issues introduced from here ;)
f0d9270

avatar wilsonge
wilsonge - comment - 22 Jan 2017

Thanks!

Add a Comment

Login with GitHub to post a comment