feat: remove internal::TimestampFromCounts() #1183
Conversation
The internal RFC3339 and protobuf conversions to/from `Timestamp`, along with the public `std::chrono::time_point` conversions, are sufficient for our testing needs. Also add a new test Timestamp.RegularSemantics, and extend test Timestamp.RelationalOperators to cover inequalities.
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #1183 +/- ##
==========================================
- Coverage 95.61% 95.57% -0.05%
==========================================
Files 175 175
Lines 13554 13567 +13
==========================================
+ Hits 12960 12967 +7
- Misses 594 600 +6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Reviewed 10 of 10 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @devbww and @devjgm)
google/cloud/spanner/timestamp_test.cc, line 89 at r1 (raw file):
TEST(Timestamp, FromRFC3339) { EXPECT_EQ(internal::TimestampFromProto(MakeProtoTimestamp(1561135942, 0)),
nit: can you document how the strings are created? A future me trying to change one of these tests would like to know.
Add a note about one mechanism for externally converting between std::time_t values and UTC strings, possibly for use when adding test cases.
There was a problem hiding this comment.
Reviewable status: 9 of 10 files reviewed, all discussions resolved (waiting on @coryan and @devjgm)
google/cloud/spanner/timestamp_test.cc, line 89 at r1 (raw file):
Previously, coryan (Carlos O'Ryan) wrote…
nit: can you document how the strings are created? A future me trying to change one of these tests would like to know.
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @devjgm)
…-cpp-spanner#1183) * feat: remove internal::TimestampFromCounts() The internal RFC3339 and protobuf conversions to/from `Timestamp`, along with the public `std::chrono::time_point` conversions, are sufficient for our testing needs. Also add a new test Timestamp.RegularSemantics, and extend test Timestamp.RelationalOperators to cover inequalities. * Address review comment Add a note about one mechanism for externally converting between std::time_t values and UTC strings, possibly for use when adding test cases.
The internal RFC3339 and protobuf conversions to/from
Timestamp,along with the public
std::chrono::time_pointconversions, aresufficient for our testing needs.
Also add a new test Timestamp.RegularSemantics, and extend test
Timestamp.RelationalOperators to cover inequalities.
This change is