[5.x] Better Feature Integration

sausin / laravel-ovh

Wrapper for OVH Object Storage integration with laravel

MIT License

37 stars 12 forks source link

[5.x] Better Feature Integration #50

Closed iksaku closed 4 years ago

iksaku commented 4 years ago

OVHSwiftAdapter::getTemporaryUrl() now throws an exception if no Temp URL Key is provided in config.
SetTempUrlKey command will now show an error message if the specified disk in --disk doesn't exist in configuration.
Moved tests to Functional Directory, and split them into more Specific Files.
Changed OVHConfiguration class to have a protected constructor (Force ::make() for checks) and have Fluent Setters for external modification (Tests mainly).

sausin commented 4 years ago

Looks good! Just wanted to discuss regarding change of containerName from container. This will be a breaking change if people have already setup their config for 5.x

I can see the clarity this provides, but what if we keep this for 6.x milestone? No breaking changes will happen this way for people already on 5.0.0 (even though it was marked as pre-release).

iksaku commented 4 years ago

Oh right, you tagged v5.0 directly as pre-release, I though you marked it as v5.0-RC1 or something like that...

Well, we’re kinda on a daily streak of PRs hehe, I think we should keep the rename up to this PR. No further “as-ovh” key name changes are needed after this anyways. Future config keys will be new anyways, so I think we can afford to push this last key change and expect some issues being open.

If you still want to keep this for a future 6.x, I can push to revert the name change and open a [6.x] issue as a reminder.

sausin commented 4 years ago

Done! Thanks :+1: