Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factor out datastore API surface. #2097

Merged
merged 1 commit into from
Aug 17, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Aug 12, 2016

This is in preparation for a gRPC equivalent of that API.

@dhermes dhermes added api: datastore Issues related to the Datastore API. grpc labels Aug 12, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 12, 2016
@tseaver
Copy link
Contributor

tseaver commented Aug 17, 2016

The API helper indirection SGTM. One question: should the tests for the Client methods change to use a mocked helper? Seems like it would make swapping in the gRPC version later more straightforward to test.

@dhermes
Copy link
Contributor Author

dhermes commented Aug 17, 2016

Does #2099 make it clear how the gRPC test swapping is done?

LGTY?

@tseaver
Copy link
Contributor

tseaver commented Aug 17, 2016

Hmmm, it looks as though we don't have explicit tests for the Connection API methods at all: we must be getting coverage via the higher-level objects?

@dhermes
Copy link
Contributor Author

dhermes commented Aug 17, 2016

It's unclear to me. Can we handle outside this PR? (Working on fixing the merge conflict right now.)

This is in preparation for a gRPC equivalent of that API.
@tseaver
Copy link
Contributor

tseaver commented Aug 17, 2016

#2113 tracks the indirect coverage issue. LGTM pending Travis.

@dhermes dhermes merged commit a2c8182 into googleapis:master Aug 17, 2016
@dhermes dhermes deleted the datastore-prep-for-grpc branch August 17, 2016 20:36
@dhermes dhermes mentioned this pull request Sep 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API. cla: yes This human has signed the Contributor License Agreement. grpc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants