? Success

User tests: Successful: Unsuccessful:

avatar itbra
itbra
6 May 2014

Detecting an image's orientation (landscape, portrait, etc) may become a repetitive task when is comes to decide which values to set for width and height to the resize() method. I found my self in need to know if the images i process are in landscape or portrait or (rather rarly) in square format. As a first step i coded this check whereever i needed it which i found pretty unhandy. Querying this information from the JImage class statically like is possible with the getImageFileProperties() method was a very helpful faster solution.

At another stage (when preparing my view) i found myself again require to know an image's orientation to apply a CSS class. As can be seen the need for this information is there which is why i add this pull request including the 3 new methods: isCubic(), isLandscape(), isPortrait().

Test Instructions (mabe somebody experienced is willing to adopt it into the image test class?

1. Create the helper class file from this PR under JPATH_LIBRARIES . '/joomla/image/helper.php'

2. In JPATH_LIBRARIES . '/joomla/image/image.php' method `getImageFileProperties()` replace

// Build the response object.
$properties = (object) array(
    'width' => $info[0],
    'height' => $info[1],
    'type' => $info[2],
    'attributes' => $info[3],
    'bits' => isset($info['bits']) ? $info['bits'] : null,
    'channels' => isset($info['channels']) ? $info['channels'] : null,
    'mime' => $info['mime']
);

return $properties;

with

return JImageHelper::getImageFileProperties($path);

as wished by @dongilbert.

3. Take any file e.g. the currently active template's index.php and at the very top 
right below `defined('_JEXEC') or die;` paste this code:

// Define the file to use. 
// NOTE:   Make sure this file exists or use another path to an existing image file!
$file = JPATH_BASE . '/images/powered_by.png';

echo '<pre>file: ' . print_r($file, true) . '</pre>';
echo '<pre>' . print_r(str_repeat('_', 51), true) . '</pre>';

// Get instance of JImage.
$image = new JImage($file); // $image must be instance of JImage

// Get image properties calling static method from JImageHelper.
$props = JImageHelper::getImageFileProperties($file);   // $props must be an object

echo '<pre>props: ' . print_r($props, true) . '</pre>';
echo '<pre>' . print_r(str_repeat('_', 51), true) . '</pre>';

echo '<pre>' . print_r('Check for every property separately:', true) . '</pre>';
echo '<pre>' . print_r(str_repeat('_', 51), true) . '</pre>';

// Check the returned data contains all of these properties.
foreach (array(
   'width',
   'height',
   'type',
   'attributes',
   'bits',
   'channels',
   'mime',
   'filesize',
   'orientation') as $p
)
{
    if (array_key_exists($p, $props))
    {
        echo "<pre>{$p} is: " . print_r($props->$p, true) . '</pre>';
    }
    else
    {
        echo '<pre>' . print_r("\$props does not contain {$p}-data", true) . '</pre>';
    }
}

echo '<pre>' . print_r(str_repeat('_', 51), true) . '</pre>';
jexit('Done');

The expected output should be:

file: /path/to/joomla/images/powered_by.png
___________________________________________________
props: stdClass Object
(
    [width] => 150
    [height] => 35
    [type] => 3
    [attributes] => width="150" height="35"
    [bits] => 8
    [channels] => 
    [mime] => image/png
    [filesize] => 2301
    [orientation] => landscape
)
___________________________________________________
Check for every property separately:
___________________________________________________
width is: 150
height is: 35
type is: 3
attributes is: width="150" height="35"
bits is: 8
channels is: 
mime is: image/png
filesize is: 2301
orientation is: landscape
___________________________________________________
Done

Any feedback is welcome.

avatar itbra itbra - open - 6 May 2014
avatar itbra itbra - change - 6 May 2014
Labels
avatar itbra itbra - change - 6 May 2014
Labels Added: ?
avatar Bakual
Bakual - comment - 6 May 2014

Just wondering if it may make more sense to have something like getOrientation() which returns landscape, portrait or cubic.
In case of the CSS class, you could even directly use the output if you want.

avatar itbra
itbra - comment - 6 May 2014

Agree. The generic method indeed provides more flexibility.
What do you think, should i replace my previous implementation or should we wait for more feedback?

avatar Bakual
Bakual - comment - 6 May 2014

I'd wait a day and see if someone else has an issue with it.

avatar betweenbrain
betweenbrain - comment - 6 May 2014

Agree. The generic method indeed provides more flexibility.

:+1:

avatar chrisdavenport
chrisdavenport - comment - 6 May 2014

Shouldn't that be "square" rather than "cubic"? I don't think we support 3D printers yet. ;-)

avatar Bakual
Bakual - comment - 6 May 2014

Shouldn't that be "square" rather than "cubic"? I don't think we support 3D printers yet. ;-)

See, that's why it's good to have native english speakers looking over it :+1:

avatar bembelimen
bembelimen - comment - 6 May 2014

Perhaps something like that would be an improvement:
https://github.com/itbra/joomla-cms/pull/2/files

I moved all the exception stuff to a helper method.

avatar piotr-cz
piotr-cz - comment - 7 May 2014

What about using just one method, so we don't have to run all three to check the orientation.
Simpler usage and shorter commit

const ORIENTATION_LANDSCAPE = 1;
const ORIENTATION_PORTAIT = 2;
const ORIENTATION_SQUARE = 4;

/**
 * @return  integer   Orientation constant
 */
public static function getOrientation($path)
{
    $info = getimagesize($path);

    if ($info[0] > $info[1])
    {
        return static::ORIENTATION_LANDSCAPE;
    }
    else if ($info[0] < $Info[1])
    {
        return static::LANDSCAPE_PORTAIT;
    }
    else
    {
        return static::LOCATION_SQUARE;
    }
}
avatar Fedik
Fedik - comment - 7 May 2014

one method looks better :+1:

avatar itbra
itbra - comment - 7 May 2014

I absolutely agree to the one-method-solution, which is why i replaced my previous implementation by a single generic method.

However, consider another situation where a user already created an instance of JImage and already has a linked resource waiting for processing (resize, crop, etc.). The user now wants to get the desired information from this resource. When implementing the getOrientation() method statically the user has no access to this method since the class doesn't inherit another class that might provide a static orientation check method. And calling our statically implemented getOrientation() method from within the instance would result in an error, because it is not possible to call a static method from within an instance via $this->getOrientation().

So it seems we either have to add a parent class (containing the static methods) which JImage inherits from or the getOrientation() method must not be implemented statically requiring to create a new JImage instance passing in the resource of the instance the user is currently working with everytime a he wants to get the desired information.

What do you think?

@piotr-cz I wonder if it may be required to have constants for the orientation as i cannot imagine any other situation where these contants might be helpful. Having numeric values for the orientation appears to be useless to me.

avatar itbra itbra - change - 7 May 2014
The description was changed
Description <p>Detecting an image's orientation (landscape, portrait, etc) is a repetitive task when is comes to decide which values to set for width and height to the resize() method. I found my self in need to know if the images i process are landscape or portrait or rather rare in cubic format. As a first step i coded this check where i needed it which i found pretty unhandy. Querying this information statically like is possible with the getImageFileProperties() method was a very helpful fast accessor for this information.</p> <p>At another stage (when preparing my view) i am again required to know an image's orientation to apply a CSS class. As can be seen the need for this information is there which is why i add this pull request including the 3 new methods: isCubic(), isLandscape(), isPortrait().</p> <p>Any feedback is welcome.</p> <p>Detecting an image's orientation (landscape, portrait, etc) may become a repetitive task when is comes to decide which values to set for width and height to the resize() method. I found my self in need to know if the images i process are in landscape or portrait or (rather rarly) in square format. As a first step i coded this check whereever i needed it which i found pretty unhandy. Querying this information from the JImage class statically like is possible with the getImageFileProperties() method was a very helpful faster solution.</p> <p>At another stage (when preparing my view) i found myself again require to know an image's orientation to apply a CSS class. As can be seen the need for this information is there which is why i add this pull request including the 3 new methods: isCubic(), isLandscape(), isPortrait().</p> <p>Any feedback is welcome.</p>
Labels Added: ? ?
avatar piotr-cz
piotr-cz - comment - 7 May 2014

@itbra Yes, I'm not sure whether to use static method or not.

I'd say the convention is to have all static method in an helper class like JImageHelper but in JImage we already have static getImageFileProperties.

Maybe static methods might be useful for processing images in bulk when creating a JImage instance for every file would be waste of resources.

Well, at the end we can have both :) Let's see what @dongilbert thinks.

As for the constants, the goal is to not use it's values, just names

switch (JImage::getOrientation($imgPath))
{
    // No integeres, but constant name
    case JImage::ORIENTATION_LANDSCAPE:
       echo "It's landscape image";
       break;

    // Landscape and portait gives square, this is when using integers might be handy
    case JImage::ORIENTATION_LANDSCAPE & JImage::ORIENTATION_PORTAIT:
        echo "Square";
}
avatar itbra
itbra - comment - 7 May 2014

I think i figured a good solution. Let me explain referring to my current situation (multi-upload of several images the same time)

Uploading an image includes several tasks like path sanitasation, input validation, upload processing.
Assuming all sanitasation succeeded i now have a bunch of tmp files waiting to be processed in $_FILES. What i now do is:

  1. Create a new instance of JImage not passing in anything to the constructor and assign it to $jimage.
  2. Iteratively save every uploaded file into a tmp folder via JFile::move($input, $output). Then call $jimage->loadFile($tmpfile). This loads the uploaded file ready for processing (resice, scale, crop, etc.).
  3. Call $jimage->getOrientation() method not passing in a parameter. This will cause the method to use the loaded resource and fetch the orientation from it.
  4. Evaluate the returned value like:
$isLandscape = $jimage->getOrientation() == JImage::ORIENTATION_LANDSCAPE;
$isPortrait = $jimage->getOrientation() == JImage::ORIENTATION_PORTRAIT;
$isSquare = $jimage->getOrientation() == JImage::ORIENTATION_SQUARE;
  1. Process the resource and save it.
if ($isLandscape)
{
   $jimage->cropResize($fullsize_width, null);
}
elseif ($isPortrait)
{
   $jimage->cropResize(null, $fullsize_height);
}
elseif ($isSquare)
{
   $jimage->cropResize($fullsize_width, $fullsize_height);
}

$jimage->toFile($targetFile);

Then, when it comes to prepare the view and assign classes for CSS access:

  1. Create a new instance of JImage not passing in anything to the constructor and assign it to $jimage.
  2. Iteratively check whether the file exists and call $jimage->loadFile($file). This loads the uploaded file ready for processing (resice, scale, crop, etc.).
  3. Dump the orientation $orientation = $jimage->getOrientation()
  4. Assign the orientation to the class attribute which will then be output.

This way i found the non-static implemenation seems to be the better solution. Also, as in either case there is only one JImage instance doing the job, there is no waste of resource as mentioned by piotr-cz above.

Your comments and arguments are highly welcome.

avatar piotr-cz
piotr-cz - comment - 7 May 2014

Let's use non-static method then.

We are already using constants for scale methods and IMHO this is a standard in programming when values are not dynamic. I could not find any resources to support this, but consider that
Imagick::getImageOrientation is using constants, and SilverStripe::Image uses same constants as the one I suggested (the API is very similar to our JImage).

As for orientation checking in your example, I guess your code would look cleaner using switch control structure.

avatar sovainfo
sovainfo - comment - 7 May 2014

May I suggest the following code:
$orientation = $jimage->getOrientation();
$width = ($orientation == 'portrait') ? null : $fullsize_width;
$height = ($orientation == 'landscape') ? null: $fullsize_height;
$jimage->cropResize($width, $height);
$jimage->toFile($targetFile);

avatar bembelimen
bembelimen - comment - 7 May 2014

I would use static class constants, so I can check:

[...]
if ($orientation == JImage::ORIENTATION_LANDSCAPE)
[...]

(and the method itself returns one of the constants)

So it doesn't matter what value (= string) the methods return or if you use "portrait" or "-1" etc. (or other stuff in future versions) it will always works

avatar itbra
itbra - comment - 7 May 2014

You convinced me. I updated my previous comment and the PR.

I think we are good to go for testing and merging.

avatar dongilbert
dongilbert - comment - 7 May 2014

Hi @itbra, good work on this so far. I really like your concept here and great job taking in feedback. I'd like to suggest a couple more improvements though, if I could.

IMO, the current getImageFileProperties() static method is not very useful, because it just re-maps the results of getimagesize($imgPath). I wonder if it would be more useful if we also added an orientation property to the retuned data from getImageFileProperties(). That would provide you with the static access to the info that you originally wanted (by providing a file path), and also remove the code duplication from the getOrientation() method. To accomplish this, I would move the logic for determining orientation to a JImageHelper static method. That method would look like (notice I moved the constants to the helper as well):

/**
 * Compare width and height integers to determine image orientation.
 *
 * @param  integer  $width
 * @param  integer  $height
 *
 * @return  mixed   Orientation string or null.
 *
 * @since   __DEPLOY_VERSION__
 */
public function getOrientation($width, $height)
{
    switch (true)
    {
        case ($width > $height) :
            return self::ORIENTATION_LANDSCAPE;

        case ($width < $height) :
            return self::ORIENTATION_PORTRAIT;

        case ($width == $height) :
            return self::ORIENTATION_SQUARE;

        default :
            return null;
    }
}

And then change getOrientation() on the JImage class to:

/**
 * Method to detect an whether image's orientation is landscape, portrait or square.
 * The orientation will be returned as string.
 *
 * @return  mixed   Orientation string or null.
 *
 * @since   __DEPLOY_VERSION__
 */
public function getOrientation()
{
    if ($this->isLoaded())
    {
        $width  = $this->getWidth();
        $height = $this->getHeight();

        return JImageHelper::getOrientation($width, $height);
    }

    return null;
}

Then, in the getImageFileProperties() method, I would add to the $properties object:

$properties = (object) array(
    //...snip
    'orientation' => JImageHelper::getOrientation((int) $info[0], (int) $info[1]);
);
avatar itbra
itbra - comment - 7 May 2014

Perfect! This solution looks smart and satifies both requirements - static access by passing in a path only and access on the loaded resource.

What do you suggest how to proceed? Do you want me to create the JImageHelper file and integrate your suggested solution or do you wanna keep your baby in your hands and integrate was we elaborated?

avatar dongilbert
dongilbert - comment - 7 May 2014

Add the static getOrientation to JImageHelper, move the class constants over, and make the suggested modifications to JImage->getOrientation() and JImage->getImageFileProperties().

avatar itbra
itbra - comment - 7 May 2014

Ok. I searched my current Joomla! installation (3.2.3) for a class JImageHelper and didn't find any, which is why i assume i have to create it. So far i never created new core files and feel little unsure. Please tell me, where you want me to create the helper file? In libraries/cms/helper as class JHelperImage ?

avatar dongilbert
dongilbert - comment - 7 May 2014

Sorry - I didn't realize the class did not already exist. You would create it at libraries/joomla/image/helper.php.

While you're creating that, can you also move the getImageFileProperties() method to it, then proxy that method in JImage. The JImage method would look like this now: (Leave the other doc-block things, but add the @deprecated as well.

/**
 * @deprecated  4.0  Use JImageHelper::getImageFileProperties instead.
 */
public static function getImageFileProperties($path)
{
    return JImageHelper::getImageFileProperties($path);
}
avatar itbra itbra - reference | - 7 May 14
avatar itbra itbra - reference | - 7 May 14
avatar itbra
itbra - comment - 7 May 2014

Ok, i'm done for the moment. Please have a look and let me know if there's something missing or wrong.

Also, what about the DEPLOY_VERSION string in the docblocks? Is this kind of a placeholder for the deploy script or would i have to substitute it by the current platform version?

avatar dongilbert
dongilbert - comment - 7 May 2014

Yes, that is a placeholder for the deployment script.

avatar itbra
itbra - comment - 7 May 2014

Works like charme now. :-)

avatar mbabker
mbabker - comment - 7 May 2014

Don, we actually don't have that deploy version placeholder setup in the
CMS.

On Wednesday, May 7, 2014, Don Gilbert notifications@github.com wrote:

Yes, that is a placeholder for the deployment script.

—
Reply to this email directly or view it on GitHub#3568 (comment)
.

avatar dongilbert
dongilbert - comment - 7 May 2014

We should fix that. :)

avatar mbabker
mbabker - comment - 7 May 2014

Since the release commit isn't done by CI, adding a script to local
workflow isn't a big deal. Just remember that the release workflow in the
CMS is more manual work than the Framework :-)

On Wednesday, May 7, 2014, Don Gilbert notifications@github.com wrote:

We should fix that. :)

—
Reply to this email directly or view it on GitHub#3568 (comment)
.

avatar Kubik-Rubik
Kubik-Rubik - comment - 9 May 2014

Good solution, should be integrated into the core.

avatar itbra
itbra - comment - 12 May 2014

@dongilbert

Would you mind to make the fill color for cropped images (not only PNGs) configurable via constructor option and thus customisable? It looks pretty ugly when the page presenting images has a white background color but the cropped image(s) have a black fill color.
I can extend the helper with a method converting color strings like used with CSS into an RGB-array if you like.

@mbabker
Do you want me to replace the placeholders by the currently used numerical platform number?

avatar piotr-cz
piotr-cz - comment - 12 May 2014

@itbra Check out my Pull Request #1562, JImageFilterBackgroundfill. Similar has been merged in Joomla-Framework, but CMS is still waiting.

There's a method sanitizeColor for color conversions. I remember Don suggested it may could be moved to a helper class which we didn't have at that moment

avatar itbra
itbra - comment - 12 May 2014

Nice. Wonder, why this hasn't found its way into the CMS yet. Processing images is an important functionality to CMS and within the last 10 months it could have been integrated and deployed i think.

avatar piotr-cz
piotr-cz - comment - 12 May 2014

I think it just have been forgotten, as the other pull request adding SCALE_FIT got in easily.

avatar itbra
itbra - comment - 13 May 2014

I added the filesize information to the returned object of JImageHelper::getImageFileProperties($path) as i found this information quite useful and required.

avatar infograf768
infograf768 - comment - 17 May 2014

I guess this could go in 3.4.0. Can we have a new Feature tracker on joomlacode and precise test instructions? Thanks.

avatar itbra itbra - change - 21 May 2014
Title
Add fast check for image orientation
[#33765] Add fast check for image orientation
avatar itbra
itbra - comment - 21 May 2014

I created the tracker item and updated this PR's title to reflect the associated tracker item id.
However, i'm afraid i can't provide with any test instructions. I don't know how to do this and depend on the help of our participants. Mabe @dongilbert can help out?

avatar piotr-cz
piotr-cz - comment - 21 May 2014

@dongilbert What is your opinion on using abstract class for helpers, like here?
I know it's a convention in the CMS (perhaps to avoid instantiation of such classes) but IMHO it's an misuse of abstract type.

avatar brianteeman brianteeman - change - 25 May 2014
Labels
avatar brianteeman brianteeman - change - 25 May 2014
Labels
avatar nicksavov nicksavov - change - 21 Aug 2014
Labels Removed: ?
avatar nicksavov nicksavov - change - 21 Aug 2014
Labels
avatar itbra
itbra - comment - 29 Aug 2014

Can we merge this to 3.4-dev? I have another PR for the helper.php in the pipe but cannot add it as long as this PR is not merged because the file doesn't exist.

avatar dbhurley
dbhurley - comment - 29 Aug 2014

Yes, if it's submitted against that branch we can test and merge into 3.4-dev

avatar itbra
itbra - comment - 29 Aug 2014

I'm not sure what you're telling me. Must i repeat all the steps above with a new PR against the 3.4-dev branch? If so, i wonder why since i created a couple other PRs against 3.3 and they've been merged into 3.4-dev. I'm not very experienced in the usage of git and am little confused now.

avatar Bakual
Bakual - comment - 29 Aug 2014

This PR against the staging branch is fine. We can always commit it to the 3.4-dev branch when merging.
I didn't see at least two good tests yet, but I may be blind.

I have another PR for the helper.php in the pipe but cannot add it as long as this PR is not merged because the file doesn't exist.

You can actually create the class with another PR as well. Once one of the PRs is merged, the other just needs to be updated as it will create a conflict. But it's not a hard thing to do.

avatar itbra
itbra - comment - 29 Aug 2014

Thanks Thomas for your "translation". I wonder nobody tested this yet. Can you start the game and test it soon? I'd like to see this deployed with 3.4.

avatar dbhurley
dbhurley - comment - 29 Aug 2014

Sorry for not speaking clearly enough. Thanks @Bakual for elaborating. ;)

avatar dbhurley
dbhurley - comment - 29 Aug 2014

I think it can be tested easily enough if you want to outline the steps an admin user would take to make sure nothing breaks. It's new code so there's no concern about backwards compatibility.

Maybe just a quick comment outlining what a user should do and what they can expect to happen after they do it. Easy!

avatar itbra
itbra - comment - 30 Aug 2014

I Updated the opening post and added some testing instructions. Note, these are no Unit-Tests. I have no knowledge in Unit-testing and no experience in writing such tests. For the sake of seeing this PR merged to 3.4-dev i ask you guys to test it the quick 'n' dirty way and report your feedback, please.

Anybody experienced in adopting this test into a Unit test is highly welcome to contribute a proper test class to the framework. I suppose this should go into a JImageHelperTest class into this folder.

avatar brianteeman brianteeman - change - 30 Aug 2014
Status New Pending
avatar itbra
itbra - comment - 3 Mar 2015

Anyone to take some minutes to test this feature? The testing procedure is well documented at the associated github item and really takes very less time.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3568.
avatar itbra
itbra - comment - 3 Mar 2015

Sorry for the wrong link. Here is the correct one.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3568.
avatar zero-24
zero-24 - comment - 9 May 2015

@itbra sorry for the delay.

I have just tested it but i'm not exactly sure if it is a successful test?

I have add your code form here: #3568 (comment)

The result is:
``file: [Joomla Root]/images/powered_by.png orientation from JImage: landscape fileProperties contains width: 1 fileProperties contains height: 1 fileProperties contains type: 1 fileProperties contains attributes: 1 fileProperties contains bits: 1 fileProperties contains channels: 1 fileProperties contains mime: 1 fileProperties contains filesize: 1 fileProperties contains orientation: 1 orientation from JImageHelper: landscape


Is this the correct one?
avatar roland-d
roland-d - comment - 14 May 2015

@test The orientation is returned as expected but I also wonder like @zero-24 if the rest of the info is correct, it looks wrong to me.

On another note, I don't understand why the code is in a helper file and not just in JImage? Is it just because so you can't get the static value of the orientation? To me, it would be nicer to just have one class handling the image information.

avatar itbra
itbra - comment - 14 May 2015

@zero-24 and @roland-d thanks for testing. I updated the test instructions to report clearly what's going on. I forgot to update them while discussing and working out all the changes.

@roland-d If you read the whole discussion you'll notice that i initially added my solution to class JImage. However, on 7 May 2014 the original author @dongilbert asked me to move it over to JImageHelper.

avatar zero-24
zero-24 - comment - 19 May 2015

@test successful with the last stat of the test instructions. Thanks @itbra

avatar zero-24 zero-24 - test_item - 19 May 2015 - Tested successfully
avatar roland-d
roland-d - comment - 28 May 2015

@itbra Thanks for the explanation, I saw that @dongilbert asked you to move it to a helper class. I was just asking what the reason behind it is. I don't see the advantage of it.

avatar itbra
itbra - comment - 28 May 2015

Neither do I. But since I just wanted to see this feature getting merged soon I just did what I was asked for. Unfortunately it isn't yet merged.

avatar roland-d
roland-d - comment - 28 May 2015

@itbra It wasn't merged because there were no tests, now there are 2 tests by myself and @zero-24 so that is OK. However I do question the usage of the helper file. Either we make it 1 file and merge it like that, otherwise I will consult with the PLT to see if it needs to be a single file or needs a helper file.

avatar itbra
itbra - comment - 28 May 2015

Whatever fits and suits the merge requirements.

avatar roland-d
roland-d - comment - 28 May 2015

@wilsonge @Bakual Can you guys please answer if the change should be in a helper file or the main JImage file? My preference goes to the main JImage file.

avatar Kubik-Rubik
Kubik-Rubik - comment - 28 May 2015

I would be fine with the helper class. +1

As a rule of thumb, helpers should contain functionality that is common but has no special designation under the overall architecture of the application.

  • Suffix the classname with Helper
  • Use static methods whenever possible

http://stackoverflow.com/questions/7064867/helper-class-in-php

Developers could easily use the helper classes in their own extensions without having to instantiate a JImage object.

avatar Bakual
Bakual - comment - 28 May 2015

I don't care either way.
JImage is more used to actually change the image in some way. While this proposed code only reads information from the image.

avatar roland-d roland-d - test_item - 28 May 2015 - Tested successfully
avatar roland-d
roland-d - comment - 28 May 2015

Thanks guys, merged in 449b907

avatar roland-d roland-d - change - 28 May 2015
The description was changed
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-05-28 14:56:21
Closed_By roland-d
avatar roland-d roland-d - close - 28 May 2015
avatar smz
smz - comment - 29 May 2015

well... I don't know and it is now surely too late, but my 2 cents are that a new helper class for 2 methods that could had fit in JImage without breaking anything is... over-engineering

Think it over.... we are creating a legacy...

avatar smz
smz - comment - 29 May 2015

... and I think all those @since __DEPLOY_VERSION__ are wrong...

avatar Bakual
Bakual - comment - 29 May 2015

@smz Can you do a PR please? It's never to late to change something as long as it isn't released yet.
The __DEPLAY_VERSION__ stuff sure is wrong.

avatar smz
smz - comment - 29 May 2015

@Bakual Of course! Will do ASAP (later today...)

avatar smz
smz - comment - 29 May 2015

New PR fixing some issues in this: #7060

Add a Comment

Login with GitHub to post a comment