Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PHP 5.4 support #11

Closed
wants to merge 3 commits into from
Closed

Add PHP 5.4 support #11

wants to merge 3 commits into from

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented May 10, 2017

I really like this library and would like to use it in selfoss RSS reader. Unfortunately, we target PHP 5.4 since it is still supported by Debian until May 2018, which many of our users run on shared web hosts.

Luckily, the library seems to work so we only need to de-bump constraints in composer.json. I also added linter to catch accidental uses of unsupported syntax.

Could you please consider supporting 5.4 for one more year?

Edit: Hmm, it looks like imagescale is not supported on PHP 5.4 in addition to HHVM. Resize could be taken from this MIT library: https://github.com/claviska/SimpleImage/blob/3158283f02712928e51ff6e2f0bfcfeb31b48e66/src/claviska/SimpleImage.php#L756

@lordelph
Copy link
Owner

Given 5.4 is 2 years past its end of life, I wouldn't want to burden this relatively new package with having to support it.

However, here is one approach we could take

  • get this pull request passing with some kind of polyfill for imagescale
  • branch current master as v2.0, so that v2.0 only supports current php releases
  • then have a second branch of master as v1.1, to which this pull request is applied

That way, anyone needing support for old php versions can specify that in their composer.json

How does that sound?

@jtojnar
Copy link
Contributor Author

jtojnar commented May 10, 2017

That would work for us. I could help backporting fixes if the code diverges too much.

@lordelph
Copy link
Owner

I'll see what I can do - might be the weekend before I can devote any time to this. Feel free to add to the PR in the meantime if you're able.

@jtojnar jtojnar force-pushed the php54 branch 6 times, most recently from 95d950c to a2c8197 Compare May 10, 2017 20:56
@jtojnar
Copy link
Contributor Author

jtojnar commented May 10, 2017

I probably have a different GD configuration on my computer because all the image comparisons fail for me. Nevertheless, I have managed to fetch the generated image from Travis and now it should finally work.

@lordelph
Copy link
Owner

This is now merged and available as v1.1.0, so if you pull this into composer with a constraint to just use 1.*, you should be good.

I've merged this into a different branch (php54), and any further backwards compatibility fixes can be made there. GitHub doesn't seem to recognize its merged, so will just close this pull request.

@lordelph lordelph closed this May 14, 2017
@jtojnar jtojnar deleted the php54 branch May 14, 2017 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants