search

thephpleague / glide

Wonderfully easy on-demand image manipulation library with an HTTP based API.
http://glide.thephpleague.com
MIT License
2.55k stars 198 forks source link

Use only Flysystem v2 #301

Closed Lustmored closed 3 years ago

Lustmored commented 3 years ago

This PR contains changes that remove Flysystem 1 and adapt to Flysystem 2.

Most of the work went into changing test suite to mimic Flysystem 2 exception logic, while changes to the code are rather simple.

I create another PR to visualize changes needed to migrate fully from V1 to V2 in case this will be the way chosen by maintainers.

tgalopin commented 3 years ago

This is a great PR, thanks a lot for your work (again!). I feel the option of releasing Glide 2.0 with only Flysystem 2 is probably the best idea, and then use 3.0 as an opportunity to add other BC breaks if necessary.

WDYT?

Lustmored commented 3 years ago

This is a great PR, thanks a lot for your work (again!). I feel the option of releasing Glide 2.0 with only Flysystem 2 is probably the best idea, and then use 3.0 as an opportunity to add other BC breaks if necessary.

WDYT?

I agree. While other PR shows that glide can support both versions, it'd probably be a burden to maintain.

tgalopin commented 3 years ago

FYI I'm merging several PRs in Glide 1.0, trying to make a final release that provides features for the ones who asked before releasing Glide 2, as it would require them to migrate to Flysystem 2.

Can you finalize this PR if you didn't already and rebase when you have time? I'll merge as soon as possible :) .

Lustmored commented 3 years ago

I have rebased to master and finalized PR as you requested. I have opted to not make any change regarding your comment as that'd require changes in tests that I am uncertain about.

Lustmored commented 3 years ago

@tgalopin just a friendly ping :)

tgalopin commented 3 years ago

Thanks! I'll merge this asap :) . Pinning the tab on my browser ;) .

rjd22 commented 3 years ago

@tgalopin I don't like hounding maintainers, because I know we're all busy. But can I help in some way with checking and merging pull-requests? I'm actively using this library.

ADmad commented 3 years ago

It would be great if there's more than one individual who has the rights to merge at least maintenance related pull request. Given the popularity of the lib and the limited amount of time the past and current maintainer has had, it takes a bit too long for PRs to be reviewed/merged. PRs such as this one in particular would be blocking development for so many projects.

tgalopin commented 3 years ago

I agree, I don't have much time to maintain this library in the short term. @ADmad would you be down for a quick chat in the coming days/weeks to discuss this? Can you ping me at galopintitouan@gmail.com ?

tgalopin commented 3 years ago

Thanks @Lustmored !

rjd22 commented 3 years ago

@tgalopin I would also not mind spending time to review, and merge PR's. If you're willing to have me.

tgalopin commented 3 years ago

@rjd22 thanks a lot for your proposal, that's very much appreciated!

I think we need to be careful not to add too many people in the team, as it can create more problems than it would solve, and in this regard the overall big involvement of @ADmad in the project led me to think we should discuss it together first. If we think we need a third person, I'll definitely ping you :) !