Make Condvar::wait_timeout return an enum instead of a bool #27826
Conversation
| /// A type indicating whether a timed wait on a condition variable returned | ||
| /// due to a time out or not. | ||
| #[derive(Debug, PartialEq, Eq)] | ||
| pub enum TimedOut { |
There was a problem hiding this comment.
This may not be the best way of representing/naming this enum.
sfackler
force-pushed
the
wait-timeout-enum
branch
from
b5d0df3 to
c0f36ab
Compare
|
Perhaps this could also have a convenience boolean accessor method like |
|
This may also want to consider naming with respect to that as well. |
alexcrichton
added
the
T-libs-api
|
I'll add a |
|
Updated with the accessor. |
|
Hm, so cc @aturon, thoughts on this return value? |
|
@alexcrichton updated |
| /// A type indicating whether a timed wait on a condition variable returned | ||
| /// due to a time out or not. | ||
| #[derive(Debug, PartialEq, Eq, Copy, Clone)] | ||
| #[unstable(feature = "wait_timeout", reason = "newly added")] |
There was a problem hiding this comment.
This'll need an explicit issue tag, and could you also add the unstable tag to the timed_out method?
|
r=me with a nit, thanks @sfackler! |
Returning a primitive bool results in a somewhat confusing API - does `true` indicate success - i.e. no timeout, or that a timeout has occurred? An explicitly named enum makes it clearer. [breaking-change]
sfackler
force-pushed
the
wait-timeout-enum
branch
from
a1b136e to
12216b7
Compare
|
@bors r=alexcrichton |
|
📌 Commit 12216b7 has been approved by |
|
⌛ Testing commit 12216b7 with merge 8951c00... |
|
💔 Test failed - auto-linux-32-opt |
|
@bors: retry |
|
⌛ Testing commit 12216b7 with merge b5f22dc... |
|
@bors: retry force |
Returning a primitive bool results in a somewhat confusing API - does `true` indicate success - i.e. no timeout, or that a timeout has occurred? An explicitly named enum makes it clearer. [breaking-change] r? @alexcrichton
Returning a primitive bool results in a somewhat confusing API - does
trueindicate success - i.e. no timeout, or that a timeout hasoccurred? An explicitly named enum makes it clearer.
[breaking-change]
r? @alexcrichton