search

provokateurin / dart-nextcloud

A Nextcloud client for dart
Other
20 stars 15 forks source link

Feature/login flow v2 #21

Closed vauvenal5 closed 3 years ago

vauvenal5 commented 3 years ago

As discussed in #20 I cherry picked the networking changes and the login flow changes from @jld3103's branch. Once this is merged I will open a 2nd PR and change the NextCloudClient host parameter from String to Uri.

provokateurin commented 3 years ago

I think bumping the version to 2.0.0 makes more sense, because it's a breaking API change and users of 1.x.x are expecting all versions to behave the same. https://dart.dev/tools/pub/versioning#semantic-versions says that the second slot is for breaking changes, but in this package it wasn't handled that way. What do you think?

vauvenal5 commented 3 years ago

Yes, I agree. However, in this case maybe it is best to also add a commit for the Uri change on API level to not have to break API in the next PR again. It is just a small change, what do you think?

provokateurin commented 3 years ago

Yes

vauvenal5 commented 3 years ago

What is the use case behind the client being able to handle a host param containing index.php?

provokateurin commented 3 years ago

I think it was just to sanitize the input and also there was a problem when you had the instance running under a path other than /, but I think you can safely remove that code.

provokateurin commented 3 years ago

Is it done?

vauvenal5 commented 3 years ago

Yes.

provokateurin commented 3 years ago

Published