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

Draft: Upgrade to intervention/image 3 (contrib) #1

Merged
merged 13 commits into from
Jan 25, 2024

Conversation

konnng-dev
Copy link

Hey

I noticed your PR on Glide's github. I'm been struggling since last year using the new intervention lib and I did some changes to make it work for my case.

Everything was well until client decided to use GIFs and animated WEBP in his website, which made my life complicated, because I vouched that Glide would be the best option to create an API to provide fast and reliable image manipulation.

Since the changes I did are quite different than yours, I decided to create this PR for you to take the files you might need to finish ours and hopefully they will approve that.

Also, as a note: unfortunately, Intervention creator decided to perform some serious breaking changes described here https://image.intervention.io/v3/introduction/upgrade#removed-features, so some features Glide uses will be have to be updated or even removed :(

@Art4
Copy link
Owner

Art4 commented Jan 24, 2024

Thank you very much for your contribution, I appreciate it very much.

I want to disable all the features that I can't get to work quickly. For now, I'm focusing on getting Intervention/Image v3 up and running with successful tests.

This way we can then look at the missing features and decide if we can drop the feature or find a replacement for it, like you did for Orientation.

@konnng-dev
Copy link
Author

Sounds like a plan

At the moment I'm working Fixing the Encoded Manipulator and its tests 👍

@Art4
Copy link
Owner

Art4 commented Jan 24, 2024

Sounds like a plan

At the moment I'm working Fixing the Encoded Manipulator and its tests 👍

I can't see a way to keep the Encode manipulator. I've deactivated it and moved the functionality into Api::run().

@konnng-dev
Copy link
Author

konnng-dev commented Jan 24, 2024

I can't see a way to keep the Encode manipulator. I've deactivated it and moved the functionality into Api::run().

You meant on tests or on actual use?

I made some new changes and fixed the tests, I think I fixed all. Could you check again?

IMO, Mockery tests are kinda tricky to get it right at first try.

@konnng-dev
Copy link
Author

@Art4 I managed to fix and test Background Modifier Properly

So, just to recap:

  • I managed to make Encode Manipulator work
  • I managed to make Background Manipulator work
  • No additional changes on Api file
  • Fixed all tests I was able to found issues

Is there something else needed? Feel free to checkout my branch and run tests to check for yourself 👍

@Art4
Copy link
Owner

Art4 commented Jan 25, 2024

Thank you very much.

I checked out your code and tried to generate an image, but got this error: Fatal error: Uncaught Error: Call to undefined method Intervention\Image\Image::getEncoded() in /src/Api/Api.php on line 104. I think the Api files needs a few adjustments.

I will try to resolve the git conflicts and merge your branch, so your contribution won't be lost.

@Art4 Art4 merged commit a819414 into Art4:upgrade-to-glide-3 Jan 25, 2024
@Art4
Copy link
Owner

Art4 commented Jan 25, 2024

I made the changes in Api::run() und resolved the conflicts.

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