search

aws / aws-dax-go

AWS DAX SDK for the Go programming language. https://aws.amazon.com/dynamodb/dax
Apache License 2.0
47 stars 48 forks source link

Addresses #3 #15

Closed aidansteele closed 4 years ago

aidansteele commented 4 years ago

Addresses #3

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

VasilyFomin commented 4 years ago

Sorry for the delayed response and thank you for the contribution. We're currently reviewing your PR.

aidansteele commented 4 years ago

Thanks for the feedback, I'll make those changes ASAP.

aidansteele commented 4 years ago

@shafeek-tk I've made those improvements as per your suggestions. Is it worthwhile also applying the same pattern to the other *PagesWithContext() methods? The others are:

Each one of the above three also has a variant that is missing WithContext().

EDIT: It occurs to me that the first two don't really make much sense to proxy through DAX. The third one does, though.

aidansteele commented 4 years ago

Thank you again for your speedy feedback. And my apologies for taking so many iterations to get this correct - clearly it's getting too late in the year for my fried brain 😄

I've addressed those changes, let me know what you think now.

ktseytlin commented 4 years ago

Hi Aidan! Thank you so much for your PR and all the work you've put into this. We had bandwidth to address this issue and pushed a fix for this in v1.1.6. Thank you!