search

a8m / documentdb

Go driver for Microsoft Azure DocumentDB
MIT License
33 stars 25 forks source link

API refactoring + features: Query parameters, API bloat reduction, Iterator, More request headers, Performace boost #15

Closed pavelsmejkal closed 5 years ago

pavelsmejkal commented 5 years ago

Hello Ariel, bumped into your project and found it interesting but some of the features we might need was missing.

I have tried to mitigate at least some of them. Unfortunately i had to break the API contract since function does returned nothing else then error.

This PR is NOT IN MERGE READY state. Tests need to be updated as well. Just wanted to discuss your point of view and if you are interested in merging possibly in future.

a8m commented 5 years ago

Hey @pavelsmejkal, Thanks so much for your contribution. At first glance, it looks very good and promising. I'll give it more attention this evening.

About breaking changes, I'm fine with bumping a new major version if it's needed.

a8m commented 5 years ago

Hey @pavelsmejkal, are you still working on this or it’s ready for review?

pavelsmejkal commented 5 years ago

Hi, @a8m you can definitely review it, i will be adding more tests. I wanted to fix the CI as well. Do you thing that go 1.3 constrain is still required ? Code is using context and it probably should use it even more. So go 1.7 would be nice to have or some interfacing work would be needed.

There is tons of things still missing like change, conflict feed support etc so when i will have a time i will try to address that as well.

a8m commented 5 years ago

Do you thing that go 1.3 constrain is still required ?

No, 1.3 was relevant when I first writing this package (~4 years ago), let's stick with latest.

You can break this into a few PRs if you want, but of course, CI should be green.

I'm going to review it in the next few hours. Thanks!

a8m commented 5 years ago

Hey @pavelsmejkal, are you still working on this?

pavelsmejkal commented 5 years ago

Sorry i am just extremely busy atm. Hopefully i have managed to cover all the comments.

pavelsmejkal commented 5 years ago

I have added as well QueryPartitionKeyRanges and refactor tests to be able to return results. Lets close the this PR. I will create another one once change feed support is bit more closer. QueryPartitionKeyRanges is just a first bite.

a8m commented 5 years ago

Thanks for the great work you did here @pavelsmejkal! I've added you as a collaborator :shipit: