search

spatie / flysystem-dropbox

A flysystem driver for Dropbox that uses the v2 API
https://freek.dev/734-dropbox-will-turn-off-v1-of-their-api-soon-its-time-to-update-your-php-application
MIT License
342 stars 50 forks source link

Ensure the path is case sensitive #10

Closed Alex-D closed 7 years ago

Alex-D commented 7 years ago

In order to fix #9 I've made a patch which can be down to dropbox-api level, but it can do some more calls to Dropbox and only do some failures with Flysystem ecosystem.

freekmurze commented 7 years ago

Don't know for certain if this going to be pulled in, but I left some comments on the code.

Alex-D commented 7 years ago

Can't use array_map or similar because I create a new associative array.

I've added a lot of comments, hope it helps you to understand my code :)

freekmurze commented 7 years ago

I really don't like the fact that the function is so long. Could you break it up into other function. Also make that function public and write some tests specifically for the function. That'll help me to further refactor the code.

Alex-D commented 7 years ago

https://github.com/spatie/flysystem-dropbox/pull/10/commits/0eac2f9bdbfa84619997ef45600401e0ee89feb0#diff-ffb6bf1e29116d4ede50555132030d1aR307

Should I split that?

And why this function should be public?

freekmurze commented 7 years ago

And why this function should be public?

To make it easier to test.

Should I split that?

Take a first stab at it. I'll refactor if necessary.

freekmurze commented 7 years ago

I'll refactor it myself. Thank you for this!

Alex-D commented 7 years ago

Sorry for the lack of time ^^' Thanks for the refactor :)