Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

feat: SqlStatement uses op<<(Value) #1361

Merged
merged 1 commit into from
Mar 12, 2020
Merged

Conversation

devjgm
Copy link
Contributor

@devjgm devjgm commented Mar 12, 2020

Nobody should use PrintTo(Value, os). This PR documents that users
should not call PrintTo and that it will be removed in an upcoming
release. All streaming of Value objects should be done using
operator<<.

Note this PR changes the output format of SqlStatement, and it
documents that the output format is for human consumption only and may
change without notice in the future (same as is documented for Value).


This change is Reviewable

@devjgm devjgm requested review from coryan and scotthart March 12, 2020 17:13
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 12, 2020
Copy link
Member

@scotthart scotthart left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 5 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @coryan)

@codecov
Copy link

codecov bot commented Mar 12, 2020

Codecov Report

Merging #1361 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1361      +/-   ##
==========================================
+ Coverage   94.53%   94.54%   +<.01%     
==========================================
  Files         186      184       -2     
  Lines       15135    15152      +17     
==========================================
+ Hits        14308    14325      +17     
  Misses        827      827
Impacted Files Coverage Δ
google/cloud/spanner/sql_statement.h 100% <ø> (ø) ⬆️
google/cloud/spanner/value.cc 96.59% <ø> (-0.05%) ⬇️
google/cloud/spanner/value.h 92.85% <100%> (-0.06%) ⬇️
google/cloud/spanner/sql_statement_test.cc 100% <100%> (ø) ⬆️
google/cloud/spanner/sql_statement.cc 100% <100%> (ø) ⬆️
google/cloud/spanner/internal/log_wrapper.h 75% <0%> (-9.62%) ⬇️
google/cloud/spanner/internal/session.h 100% <0%> (ø) ⬆️
google/cloud/spanner/internal/transaction_impl.h 100% <0%> (ø) ⬆️
...cloud/spanner/internal/partial_result_set_resume.h 100% <0%> (ø) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88adb64...196e8a8. Read the comment docs.

Copy link
Member

@coryan coryan left a comment

Choose a reason for hiding this comment

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

Nobody should use PrintTo(Value, os). This PR documents that users
should not call PrintTo and that it will be removed in an upcoming
release.

This PR is actually removing PrintTo() was that intentional?

Reviewable status: 5 of 6 files reviewed, all discussions resolved (waiting on @coryan)

Copy link
Member

@coryan coryan left a comment

Choose a reason for hiding this comment

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

Oh I see, the symbol is gone, but the inline function is there. :lgtm:

Reviewed 1 of 1 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

google/cloud/spanner/value.h Outdated Show resolved Hide resolved
google/cloud/spanner/sql_statement.h Outdated Show resolved Hide resolved
@devjgm devjgm force-pushed the rm-printto branch 2 times, most recently from e39c9e7 to 0fc7544 Compare March 12, 2020 19:47
Nobody should use `PrintTo(Value, os)`. This PR documents that users
should not call `PrintTo` and that it will be removed in an upcoming
release. All streaming of `Value` objects should be done using
`operator<<`.

Note this PR changes the output format of `SqlStatement`, and it
documents that the output format is for human consumption only and may
change without notice in the future (same as is documented for Value).
Copy link
Member

@coryan coryan left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @devbww)

@devjgm devjgm merged commit 7b526b4 into googleapis:master Mar 12, 2020
@devjgm devjgm deleted the rm-printto branch March 13, 2020 13:36
devjgm added a commit to devjgm/google-cloud-cpp that referenced this pull request May 7, 2020
…anner#1361)

Nobody should use `PrintTo(Value, os)`. This PR documents that users
should not call `PrintTo` and that it will be removed in an upcoming
release. All streaming of `Value` objects should be done using
`operator<<`.

Note this PR changes the output format of `SqlStatement`, and it
documents that the output format is for human consumption only and may
change without notice in the future (same as is documented for Value).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants