Closed jdevinemt closed 8 months ago
MaestroError
commented
8 months ago I am already margin it @jdevinemt. I guess, you forgot to add php 7.4 in the requirements but no problem, you will able to add it in the next PR 👍
P.s. Are you sure your issue #38 is resolved with these commits?
jdevinemt
commented
8 months ago I guess, you forgot to add php 7.4 in the requirements but no problem, you will able to add it in the next PR 👍
Oops, I guess I didn't commit that for some reason. I'll include it in a future PR when I have time to contribute a bit more.
P.s. Are you sure your issue #38 is resolved with these commits?
Yes, I tested this in the environment that was causing the issue and verified that it was resolved.
Do you think you'll be creating a new release soon? I'm deciding whether to hold off on deploying some changes on a project until this fix is published.
MaestroError
commented
8 months ago Since you need it 😄 I drafted a new release just now https://github.com/MaestroError/php-heic-to-jpg/releases/tag/v1.0.6 You can already update it via composer 👍 Thanks again for your contribution @jdevinemt ❤️
The fix is just a few lines in the
HeicToJpg::processImage()method. I verified that it resolved my issue #38.The issue might be specific to the linux-arm64 binary because I wasn't able to replicate the issue when I was testing locally on my mac.
Most of the commits here are general housekeeping.
str_contains()with a very simple custom method since this package did not require a specific version of PHP, butstr_contains()requires >=8.0.str_contains()and set the minimum required version to 8.0, but there is no point in limiting who can use this package further than necessary. (Especially because I am still in the process of upgrading to 8.0 at work 😅)If this PR gets merged I might contribute some other improvements if you're interested.