----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56447/#review165031 -----------------------------------------------------------
src/common/protobuf_utils.hpp (line 109) <https://reviews.apache.org/r/56447/#comment236872> hmm. calling this an "empty check status info" is confusing, because it has type set! i doubt someone reading this helper function name would know what's happening. if this is only used by health checker library, please stick it inside src/checks/utils.hpp; this doesn't seem to be generally useful. src/common/protobuf_utils.cpp (line 280) <https://reviews.apache.org/r/56447/#comment236873> looks like this function can just take CheckInfo::Type instead of CheckInfo. src/common/protobuf_utils.cpp (line 297) <https://reviews.apache.org/r/56447/#comment236874> we avoid using "default" in switch statements because it's not obvious for a new dev that this site needs to be updated when a new enum is introduced. if we don't use default, it will fail at compile time which is great! instead, you need to add a case for CheckInfo::Unknown. If this helper is not expected to be called with that case, you can just do a CHECK. Don't do `UNREACHABLE` in helpers because it should be only used when it's programatically not possible. - Vinod Kone On Feb. 8, 2017, 4:56 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56447/ > ----------------------------------------------------------- > > (Updated Feb. 8, 2017, 4:56 p.m.) > > > Review request for mesos, Gastón Kleiman and Vinod Kone. > > > Bugs: MESOS-6906 > https://issues.apache.org/jira/browse/MESOS-6906 > > > Repository: mesos > > > Description > ------- > > If a check for a task has been defined, the corresponding > `check_status` field in each task status must be set to a > valid `CheckStatusInfo` message even if there is no check > status available yet. > > > Diffs > ----- > > src/common/protobuf_utils.hpp 3ba689f30ae080ea9b2a0af4d819dd3e377c7bf5 > src/common/protobuf_utils.cpp ed84e9ae083c2dc6cd8b49d9ce8dc624a8607b8a > > Diff: https://reviews.apache.org/r/56447/diff/ > > > Testing > ------- > > See https://reviews.apache.org/r/56218/ > > > Thanks, > > Alexander Rukletsov > >
