feat!: change the result type for timers to StatusOr #134
Conversation
The result of the `future` returned from `MakeDeadlineTimer` and `MakeRelativeTimer` is now a `StatusOr` that indicates whether the timer ran to expiration or not (e.g. it was Cancelled). Part of #129
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #134 +/- ##
==========================================
+ Coverage 96.36% 96.36% +<.01%
==========================================
Files 82 82
Lines 3655 3660 +5
==========================================
+ Hits 3522 3527 +5
Misses 133 133
Continue to review full report at Codecov.
|
| @@ -70,11 +73,15 @@ TEST(CompletionQueueTest, ShutdownWithPending) { | |||
| using ms = std::chrono::milliseconds; | |||
|
|
|||
| future<void> timer; | |||
| future<void> timer2; | |||
| @@ -54,8 +54,10 @@ class CompletionQueue { | |||
| * @param deadline when should the timer expire. | |||
| * | |||
| * @return a future that becomes satisfied after @p deadline. | |||
| * The result of the future is the time at which it expired, or an error | |||
| * Status if the timer did not run to expiration(e.g. it was Cancelled). | |||
There was a problem hiding this comment.
Nit: s/(e.g. it was Cancelled)/ (e.g., it was cancelled)/
There was a problem hiding this comment.
Looks good once you address the comments that @devbww made.
Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @devbww and @mr-salty)
google/cloud/grpc_utils/completion_queue.h, line 58 at r1 (raw file):
Previously, devbww (Bradley White) wrote…
Nit: s/(e.g. it was Cancelled)/ (e.g., it was cancelled)/
nit: we also indent follow on lines, e.g.:
* @return a future that becomes satisfied after @p deadline or if
* an error if the timer fails to run until expiration.
google/cloud/grpc_utils/completion_queue.h, line 80 at r1 (raw file):
* * @return a future that becomes satisfied after @p duration time has elapsed. * The result of the future is the time at which it expired, or an error
nit: indent after @return
google/cloud/grpc_utils/completion_queue_test.cc, line 102 at r1 (raw file):
done.set_value(); }); for (int i = 0; i < 3; ++i) {
nit: i != 3 ?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @devbww)
google/cloud/grpc_utils/completion_queue.h, line 58 at r1 (raw file):
Previously, coryan (Carlos O'Ryan) wrote…
nit: we also indent follow on lines, e.g.:
* @return a future that becomes satisfied after @p deadline or if * an error if the timer fails to run until expiration.
Done (both). Would be nice if clang-format could do the latter but I don't see any options like that.
google/cloud/grpc_utils/completion_queue.h, line 80 at r1 (raw file):
Previously, coryan (Carlos O'Ryan) wrote…
nit: indent after
@return
Done
google/cloud/grpc_utils/completion_queue_test.cc, line 76 at r1 (raw file):
Previously, devbww (Bradley White) wrote…
Unused?
done (oops, it was an experiment)
google/cloud/grpc_utils/completion_queue_test.cc, line 102 at r1 (raw file):
Previously, coryan (Carlos O'Ryan) wrote…
nit:
i != 3?
I'm gonna have to call that "personal preference" instead of "nit"...
|
PTAL |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @devbww)
…le-cloud-cpp-common#134) The result of the `future` returned from `MakeDeadlineTimer` and`MakeRelativeTimer` is now a `StatusOr` that indicates whether the timer ran to expiration or not (e.g. it was cancelled). Part of googleapis/google-cloud-cpp-common#129
…le-cloud-cpp-common#134) The result of the `future` returned from `MakeDeadlineTimer` and`MakeRelativeTimer` is now a `StatusOr` that indicates whether the timer ran to expiration or not (e.g. it was cancelled). Part of googleapis/google-cloud-cpp-common#129
…le-cloud-cpp-common#134) The result of the `future` returned from `MakeDeadlineTimer` and`MakeRelativeTimer` is now a `StatusOr` that indicates whether the timer ran to expiration or not (e.g. it was cancelled). Part of googleapis/google-cloud-cpp-common#129
The result of the
futurereturned fromMakeDeadlineTimerandMakeRelativeTimeris now aStatusOrthat indicates whether thetimer ran to expiration or not (e.g. it was Cancelled).
BREAKING CHANGE: Most callers of
MakeDeadlineTimerorMakeRelativeTimerwill need to be updated.Part of #129
This change is