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.

Sign up for GitHub

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

Jump to bottom

spanner: make getTransaction public #2344

Merged
merged 2 commits into from
Jun 26, 2017
Merged

spanner: make getTransaction public #2344

merged 2 commits into from
Jun 26, 2017

Conversation

callmehiphop
Copy link
Contributor

Closes #2152

@callmehiphop callmehiphop added the api: spanner Issues related to the Spanner API. label May 31, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 31, 2017
@bjwatson bjwatson requested a review from vkedia May 31, 2017 21:30
@bjwatson
Copy link

Thanks @callmehiphop! Adding @vkedia as a reviewer, since he understands this code better than I do.

@bjwatson bjwatson requested a review from swcloud May 31, 2017 21:31
@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.988% when pulling 45bbb6f on callmehiphop:dg--spanner-txn-promises into 748816f on GoogleCloudPlatform:master.

vkedia
vkedia reviewed Jun 6, 2017
View reviewed changes
packages/spanner/src/database.js Outdated
*
* @param {object=} options - Transaction options.
* @param {boolean} options.readOnly - Specifies if the transaction is read
* only.

This comment was marked as spam.

Sign in to view

packages/spanner/src/database.js Outdated
* Get a read/write ready Transaction object.
*
* @param {object=} options - Transaction options.
* @param {boolean} options.readOnly - Specifies if the transaction is read

This comment was marked as spam.

Sign in to view

packages/spanner/src/database.js Outdated

transaction.begin(function(err) {
if (err) {
callback(err);

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@bjwatson
Copy link

@callmehiphop Can you close this out this week?

@callmehiphop
Copy link
Contributor Author

@bjwatson yes! Sorry, I've been side tracked by other things.

@bjwatson
Copy link

Thanks @callmehiphop!

@bjwatson
Copy link

@swcloud FYI, we want to get this done this week.

@callmehiphop
Copy link
Contributor Author

@stephenplusplus @vkedia this should be good to go now!

stephenplusplus
stephenplusplus suggested changes Jun 26, 2017
View reviewed changes
Copy link
Contributor

@stephenplusplus stephenplusplus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

packages/spanner/src/database.js
@@ -400,6 +400,77 @@ Database.prototype.getSchema = function(callback) {
};

/**
* Get a read/write ready Transaction object.
*
* @param {object=} options - [Transaction options](https://cloud.google.com/spanner/docs/timestamp-bounds).

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@stephenplusplus stephenplusplus merged commit ca0d96c into googleapis:master Jun 26, 2017
This was referenced Jul 5, 2017
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vkedia vkedia vkedia left review comments

@stephenplusplus stephenplusplus stephenplusplus requested changes

@bjwatson bjwatson Awaiting requested review from bjwatson

@swcloud swcloud Awaiting requested review from swcloud

Assignees
No one assigned
Labels
api: spanner Issues related to the Spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@callmehiphop @bjwatson @coveralls @stephenplusplus @vkedia @googlebot