? ? Success

User tests: Successful: Unsuccessful:

avatar laoneo
laoneo
19 Jun 2017

This pr replaces JImage with the fw package. It keeps the classe JImage because of BC to load the JImageFilter* classes.

avatar laoneo laoneo - open - 19 Jun 2017
avatar laoneo laoneo - change - 19 Jun 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Jun 2017
Category External Library Libraries
fb71c64 19 Jun 2017 avatar laoneo cs
avatar laoneo laoneo - change - 19 Jun 2017
Labels Added: ? ?
avatar mbabker
mbabker - comment - 19 Jun 2017

We've already done this in 4.0. Any reason it needs to come into 3.8?

Also, without backporting some other logging changes, we're losing some internal logging functionality in a non-B/C way here (the Framework uses a PSR-3 logger, the CMS classes use JLog, 4.0 has a bridge class for this).

avatar laoneo
laoneo - comment - 19 Jun 2017

Missed the logging part, but there can be deprecated all filters and class mapped. There is no need to have them all still in place or do I miss something.
Beside that, it would make sense to have that in 3 already available. Makes the life of us extension developers easier if it would be available in the 3 series already.

avatar mbabker
mbabker - comment - 19 Jun 2017

4.0 will have and keep all of the JImage classes, so really the only convenience factor to port it all back to 3.8 would be if you are one of the eager ones and want to use all the namespaced stuff and drop older 3.x support.

Even if we backport the logging bridge, to be honest, because of the logging API differences I don't know if we can actually consider this one for 3.x because there is an internal B/C break in how errors are logged. If JImage were PSR-3 aware then we'd be fine, but sadly it's not. The rest of the API should be fine if I'm not mistaken, but that one implementation detail really makes this hard to accept.

avatar laoneo
laoneo - comment - 20 Jun 2017

Would making JImage PSR-3 aware a BC break?

avatar mbabker
mbabker - comment - 20 Jun 2017

Making JImage PSR-3 aware helps, but you also need the delegating logger.

If you are adamant about seeing this through, in essence you will need to do the same thing as done in #12144 for the image package stuff (we can look at class mapping after that PR is applied against 3.8, but for now I would not map anything and part of my justification can be seen in the class constructors) and the parts of 9491b57 implementing the PSR-3 support bridge (the $context array in the log entry, the delegated logger class, and the ability to create it within JLog).

avatar laoneo
laoneo - comment - 20 Jun 2017

Ok, will prepare a pr with the logger backport then.

avatar laoneo
laoneo - comment - 28 Jun 2017

I'v added the logger code similar to the 4.0 branch. Additionally I have namespaced the deprecated classes for the sake of consistency.

avatar laoneo
laoneo - comment - 28 Jun 2017

If @mbabker or @wilsonge can create a tag on the image package, then I can update the composer file to point to the tag and not dev-master.

feeb720 28 Jun 2017 avatar laoneo ups
avatar mbabker
mbabker - comment - 28 Jun 2017

1.4.0 tagged

avatar laoneo
laoneo - comment - 28 Jun 2017

Thanks. Changed composer to use the version 1.4.

avatar joomla-cms-bot joomla-cms-bot - change - 27 Jul 2017
Category External Library Libraries External Library Composer Change Libraries
avatar laoneo
laoneo - comment - 10 Aug 2017

Any plan with this one?

avatar mbabker
mbabker - comment - 10 Aug 2017

Technically we shouldn't be merging this during beta. But if this is the last namespacing/FW PR we have for 3.8 then once this is sync'd to staging let's do it.

avatar laoneo laoneo - change - 10 Aug 2017
Labels Added: ?
Removed: ?
avatar laoneo
laoneo - comment - 10 Aug 2017

Ok, done

avatar joomla-cms-bot joomla-cms-bot - change - 10 Aug 2017
Category External Library Libraries Composer Change Administration com_admin External Library Composer Change Libraries
avatar mbabker mbabker - change - 10 Aug 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-08-10 13:02:04
Closed_By mbabker
avatar mbabker mbabker - close - 10 Aug 2017
avatar mbabker mbabker - merge - 10 Aug 2017
avatar wilsonge
wilsonge - comment - 3 Sep 2017

When merging into 4 - this conflicted like hell with #12144 (michael's first attempt at moving JImage to the framework packages). I've kept what he's done for now but just namespaced it. But if you want to do a follow up to standardise feel free.

avatar laoneo
laoneo - comment - 5 Sep 2017

Will do when staging is merged into 4.

Add a Comment

Login with GitHub to post a comment