Update bundle to support Flysystem v2 #59
Conversation
|
Thanks a lot @Lustmored ! It seems you should probably rebase on master as the first commits in your PR are newly merged commits. I'll review this asap :) . |
Lustmored
force-pushed
the
flysystem-v2
branch
from
98ee76b to
63fa108
Compare
Right! I have force-pushed rebase onto master. |
|
There still seems to be a merge problem, are you sure you rebased properly? |
|
I am pretty sure. Yet as I've stated - this PR is based on 'flysystem-v2' branch from original repository and targets it. I believe rebase of 'flysystem-v2' branch onto master will fix the conflicts. Alternatively I can target 'master' branch. |
|
Ah right! My bad, I must have badly rebased myself then :) I'll have a look asap (I'm preparing the SymfonyWorld conference right now, kinda busy but I'll have more time later :) ) |
|
I have changed PR target to I have also updated docs and re-added cached storage driver based on |
|
Thanks for the ping, I'll have a look :) |
Lustmored
force-pushed
the
flysystem-v2
branch
from
6af3083 to
e2a13ec
Compare
|
@tgalopin Just a friendly remainder, no pressure intended :) I've found out my commits authoring was off, so fixed it and force pushed. No changes in code this time. |
There was a problem hiding this comment.
Hi!
Thanks for the friendly ping :) . Here is a review. Also, you should rebase your branch on master to get the latest CI fixes.
More generally: I would find it useful to keep Azure and Google Cloud support. Are there credible community alternatives for Flysytem 2?
composer.json
Outdated
| "league/flysystem-ziparchive": "^1.0", | ||
| "league/flysystem-aws-s3-v3": "^2.0", | ||
| "league/flysystem-memory": "^2.0", | ||
| "lustmored/flysystem-v2-simple-cache-adapter": "^0.1.0", |
There was a problem hiding this comment.
There is no cache adapter anymore? Perhaps would it make sense to remove the adapter then?
There was a problem hiding this comment.
There is no upstream adapter for v2 and there seems to be no work towards it. V1 adapter also was very memory inefficient (https://github.com/thephpleague/flysystem-cached-adapter/issues/23). That is why I have created this simple alternative for v2, which is very helpful in my projects already, so I think it can be helpful for others too.
But of course if you feel like dropping it from the bundle, I can do so.
There was a problem hiding this comment.
Let's drop it for now, we'll add it if someone find it useful (not sure it is)
There was a problem hiding this comment.
OK, I have removed it from code and docs
I don't know how to force port to be of type integer
Lustmored
force-pushed
the
flysystem-v2
branch
from
e2a13ec to
40a377f
Compare
|
I have updated PR with Google Cloud support, rebase to master and changes requested in comments |
|
Thanks so much @Lustmored ! I'll have a look later today but thanks for your work :) ! |
|
@Lustmored, @tgalopin is there anything that you're missing, could use some help with here? I was planning to add flysystem to an existing Symfony project, but decided to postpone until this bundle starts can support V2. I am happy to put in some work in order to speed it up |
We have to wait until @tgalopin has time to take another look and possibly merge and release |
|
Thanks so much for your work @Lustmored ! I'll release 2.0 soon. |
|
Released @nCrazed could you test to ensure everything is good on your side :) ? I tested on my projects but more tests can't hurt. |
|
I am planning to start the refactoring/integration next week. Will report any problems that I run into. |
|
Thanks @Lustmored, really appreciate the help! |
|
Hi @tgalopin, @Lustmored |
I have built upon flysystem-v2 branch and set it as a target as probably such a change will require major version bump.
Most of the work was updating tests to actually run against new architecture. I have commented and removed functionality and adapters that are missing from v2.
I haven't touched docs and didn't add support for any new options as I wanted to get feedback before eventually delving more.
This PR also includes merging master into flysystem-v2 as a base for work.