8000 samples: add standalone numeric samples for Spanner by olavloite · Pull Request #3707 · GoogleCloudPlatform/java-docs-samples · GitHub
[go: up one dir, main page]
Skip to content

samples: add standalone numeric samples for Spanner #3707

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
Merged
merged 5 commits into from
Sep 22, 2020
Merged

samples: add standalone numeric samples for Spanner #3707

merged 5 commits into from
Sep 22, 2020

Conversation

olavloite
Copy link
Contributor

Adds standalone samples for the new NUMERIC data type for Spanner.

@olavloite olavloite requested a review from a team September 11, 2020 16:18
@olavloite olavloite requested a review from skuruppu September 11, 2020 16:18
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 11, 2020
lesv
lesv approved these changes Sep 11, 2020
View reviewed changes
Copy link
Contributor
@lesv lesv left a comment

Choose a reason for hiding this comment

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

Consider removing the catch in AddNumericColumnSample.java

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Sep 12, 2020
skuruppu
skuruppu approved these changes Sep 15, 2020
View reviewed changes
Copy link
Contributor
@skuruppu skuruppu left a comment

Choose a reason for hiding this comment

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

Looks good. I also want @larkee to verify as well.

@skuruppu skuruppu requested a review from larkee September 15, 2020 03:10
@skuruppu skuruppu added the api: spanner Issues related to the Spanner API. label Sep 15, 2020
larkee
larkee requested changes Sep 15, 2020
View reviewed changes
Copy link
Contributor
@larkee larkee left a comment

Choose a reason for hiding this comment

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

LGTM overall. There were just a few updates to the spec that need to be reflected here.

@olavloite olavloite requested a review from larkee September 15, 2020 16:53
larkee
larkee approved these changes Sep 21, 2020
View reviewed changes
Copy link
Contributor
@larkee larkee left a comment

Choose a reason for hiding this comment

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

LGTM once the table name in the update output strings are updated 👍

olavloite and others added 2 commits September 21, 2020 18:18
@olavloite @larkee
fix: print Venues instead of Records
e1fc6e1 
Co-authored-by: larkee <31196561+larkee@users.noreply.github.com>
@olavloite @larkee
fix: print Venues instead of Records
3af6c0e 
Co-authored-by: larkee <31196561+larkee@users.noreply.github.com>
@skuruppu
Copy link
Contributor

It seems that the Static analysis failures are always happening due to the way the samples are written. @lesv would it be ok to merge in this PR?

Also, going forwards, @olavloite are there fixes that we could make such that we don't fail this Kokoro job?

@lesv
Copy link
Contributor
lesv commented Sep 22, 2020

Static analysis often reports unfixable things. I'll be trying to fix this (reduce the noise) tomorrow. That said, it's not a required check, so feel free to submit.

@lesv lesv merged commit 1b93c01 into GoogleCloudPlatform:master Sep 22, 2020
@lesv
Copy link
Contributor
lesv commented Sep 22, 2020

I submitted for you. Just realized you never do it.

@olavloite
Copy link
Contributor Author

Also, going forwards, @olavloite are there fixes that we could make such that we don't fail this Kokoro job?

@skuruppu Some are not really fixable, while others probably are. But what in general would be needed is to split the current SpannerSamples into individual files according to the most recent style guide, and then (once the unnecessary noise has been removed) fix the warnings that are relevant.

@olavloite olavloite deleted the spanner-standalone-numeric-samples branch September 22, 2020 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skuruppu skuruppu skuruppu approved these changes

@lesv lesv lesv approved these changes

@larkee larkee larkee approved these changes

Assignees

@lesv lesv

Labels
api: spanner Issues related to the Spanner API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@olavloite @skuruppu @lesv @larkee
0