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

feat: taught Date how to stream itself #1385

Merged
merged 3 commits into from
Mar 16, 2020
Merged

Conversation

devjgm
Copy link
Contributor

@devjgm devjgm commented Mar 15, 2020

Date was the only non-aggregate "Value" type that didn't already know
how to print itself. Everything else, (bool, int64_t, double,
std::string, Bytes, and Timestamp) were all streamable directly.
internal/date.h already had this support, so it just took a little
plumbing to make the user-facing Date class to itself also be
streamable.


This change is Reviewable

Date was the only non-aggregate "Value" type that didn't already know
how to print itself. Everything else, (bool, int64_t, double,
std::string, Bytes, and Timestamp) were all streamable directly.
`internal/date.h` already had this support, so it just took a little
plumbing to make the user-facing `Date` class to itself also be
streamable.
@devjgm devjgm requested a review from devbww March 15, 2020 20:20
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 15, 2020
Copy link
Contributor

@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 r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @devbww and @devjgm)


google/cloud/spanner/date.h, line 44 at r1 (raw file):

  /// @name Output streaming
  friend std::ostream& operator<<(std::ostream& os, Date const& date);

Missing #include <ostream>?

google/cloud/spanner/date.h Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 15, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1385      +/-   ##
==========================================
+ Coverage   94.58%   94.59%   +<.01%     
==========================================
  Files         185      185              
  Lines       15316    15325       +9     
==========================================
+ Hits        14487    14497      +10     
+ Misses        829      828       -1
Impacted Files Coverage Δ
google/cloud/spanner/date.h 100% <ø> (ø) ⬆️
google/cloud/spanner/date_test.cc 100% <100%> (ø) ⬆️
google/cloud/spanner/date.cc 100% <100%> (ø) ⬆️
...loud/spanner/internal/partial_result_set_resume.cc 90.9% <0%> (-4.33%) ⬇️
google/cloud/spanner/internal/polling_loop.h 91.89% <0%> (-2.71%) ⬇️
...loud/spanner/internal/partial_result_set_source.cc 93.24% <0%> (-1.36%) ⬇️
google/cloud/spanner/internal/spanner_stub.cc 65.65% <0%> (-1.02%) ⬇️
google/cloud/spanner/client.cc 96.91% <0%> (-0.62%) ⬇️
...on_tests/rpc_failure_threshold_integration_test.cc 85.55% <0%> (-0.16%) ⬇️
google/cloud/spanner/internal/session.h 100% <0%> (ø) ⬆️
... and 10 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 4e9b0ad...c02cba7. Read the comment docs.

@devjgm devjgm merged commit 1db73f3 into googleapis:master Mar 16, 2020
@devjgm devjgm deleted the stream-date branch March 16, 2020 00:24
devjgm added a commit to devjgm/google-cloud-cpp that referenced this pull request May 7, 2020
…panner#1385)

* feat: taught Date how to stream itself

Date was the only non-aggregate "Value" type that didn't already know
how to print itself. Everything else, (bool, int64_t, double,
std::string, Bytes, and Timestamp) were all streamable directly.
`internal/date.h` already had this support, so it just took a little
plumbing to make the user-facing `Date` class to itself also be
streamable.

* added missing include

* removed friendship from op<<
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.

4 participants