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

Remove bogus .value after calling toJSON() #314

Closed
wants to merge 3 commits into from

Conversation

vrana
Copy link

@vrana vrana commented Feb 15, 2017

No description provided.

@jmdobry jmdobry requested a review from ace-n February 15, 2017 18:27
@ace-n
Copy link
Contributor

ace-n commented Mar 6, 2017

Hi @vrana and thank you for the PR.

AFAICT, those value properties are necessary - the value of the json.SingerId object for instance is { [Number: 1] value: '1' }. Since we only want to print 1, we have to get the value property of that object.

The reason why we can't have json.SingerId === 1 is because Node doesn't support 64-bit integers by default (i.e. INT64s), while Spanner does. As a workaround, the Spanner client libraries use strings to store the value of an INT64. This object is necessary to differentiate between 1 (an INT64) and "1" (a String).

@ace-n
Copy link
Contributor

ace-n commented Mar 6, 2017

@vrana Also, I tried running this on my machine and noticed that the tests failed. In the future, can you make sure to run the tests before submitting a PR? (Assuming of course that this wasn't just me - if it was, please disregard.)

(You're welcome to make changes to the tests if needed - the main thing for us is that they pass.)

@vrana
Copy link
Author

vrana commented Mar 7, 2017

This pull-request is bogus, sorry about it.

@vrana vrana closed this Mar 7, 2017
ahrarmonsur pushed a commit that referenced this pull request Nov 8, 2022
pattishin pushed a commit that referenced this pull request Nov 9, 2022
* chore(main): release 3.1.2

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
pattishin pushed a commit that referenced this pull request Nov 9, 2022
* chore(main): release 3.1.2

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
pattishin pushed a commit that referenced this pull request Nov 9, 2022
* chore(main): release 3.1.2

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
ace-n pushed a commit that referenced this pull request Nov 10, 2022
NimJay pushed a commit that referenced this pull request Nov 11, 2022
* chore(main): release 3.1.2

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
ace-n pushed a commit that referenced this pull request Nov 14, 2022
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
ace-n pushed a commit that referenced this pull request Nov 21, 2022
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
ace-n pushed a commit that referenced this pull request Nov 21, 2022
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
Shabirmean pushed a commit that referenced this pull request Feb 16, 2023
* updated CHANGELOG.md [ci skip]

* updated package.json [ci skip]

* updated samples/package.json [ci skip]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants