-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
samples: add standalone numeric samples for Spanner #3707
Conversation
There was a problem hiding this 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
spanner/cloud-client/src/main/java/com/example/spanner/AddNumericColumnSample.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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.
There was a problem hiding this 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.
spanner/cloud-client/src/main/java/com/example/spanner/InsertNumericDataSample.java
Outdated
Show resolved
Hide resolved
spanner/cloud-client/src/main/java/com/example/spanner/InsertNumericDataSample.java
Outdated
Show resolved
Hide resolved
spanner/cloud-client/src/main/java/com/example/spanner/InsertNumericDataSample.java
Outdated
Show resolved
Hide resolved
spanner/cloud-client/src/main/java/com/example/spanner/InsertNumericDataSample.java
Outdated
Show resolved
Hide resolved
spanner/cloud-client/src/main/java/com/example/spanner/InsertNumericDataSample.java
Outdated
Show resolved
Hide resolved
spanner/cloud-client/src/test/java/com/example/spanner/SpannerStandaloneExamplesIT.java
Show resolved
Hide resolved
spanner/cloud-client/src/main/java/com/example/spanner/InsertNumericDataSample.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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 👍
spanner/cloud-client/src/main/java/com/example/spanner/UpdateNumericDataSample.java
Outdated
Show resolved
Hide resolved
spanner/cloud-client/src/test/java/com/example/spanner/SpannerStandaloneExamplesIT.java
Outdated
Show resolved
Hide resolved
Co-authored-by: larkee <31196561+larkee@users.noreply.github.com>
Co-authored-by: larkee <31196561+larkee@users.noreply.github.com>
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? |
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. |
I submitted for you. Just realized you never do it. |
@skuruppu Some are not really fixable, while others probably are. But what in general would be needed is to split the current |
Adds standalone samples for the new
NUMERIC
data type for Spanner.