Gaufrette icon indicating copy to clipboard operation
Gaufrette copied to clipboard

[Flystem] Add FlysystemV2 adapter for version 2 support of flysystem.

Open jenkoian opened this issue 3 years ago • 5 comments

Done as a new adapter to allows users to use the existing adapter for flysystem version 1.

--

Let me know if are ok with the V2 adapter or if you think we should just overwrite the existing one.

Haven't touched any phpstan or psalm config, will examine the output of the github actions but may need some pointers here.

jenkoian avatar Jun 30 '21 18:06 jenkoian

Nice job @jenkoian, I'm interest in this adapter, maybe I can help,

league/flysystem:^1.0 seems to be fixed also in appveyor.yml and the Makefile (used by PHPStan analysis). Because you kept the flysystem version 1 for compatibilty (which is a good choice in my opinion), you cannot depend on both versions. Maybe the old Adapter may be excluded from PHPStan analysis ? So the requirement could be replaced by league/flysystem:^2.0.

The PHP tests seems to be failed because of trying to install olders versions of PHP (7.1, 7.2, 7.3) on ubuntu-latest, maybe an older version of ubuntu should be required for theses tests.

jum4 avatar Sep 23 '21 18:09 jum4

Thanks @jum4 so it sounds like keeping this as a v2 and keeping the existing adapter is the way to go. In which case, yeah as you mention, it looks like the static analysis isn't going to work for both. Unless we have explicit static analysis just for FSv2 which seems like overkill, or, as you mention, we only statically analyse the newer version.

So, I'm happy to update this PR, but is anyone able to confirm the direction?

jenkoian avatar Sep 24 '21 09:09 jenkoian

@nicolasmure

jum4 avatar Sep 24 '21 13:09 jum4

Version 3 of Flysystem out now too https://github.com/thephpleague/flysystem/releases/tag/3.0.0

jenkoian avatar Jan 14 '22 16:01 jenkoian

Finally done in https://github.com/KnpLabs/Gaufrette/pull/688

dmitryuk avatar Nov 25 '22 08:11 dmitryuk