> On June 7, 2016, 3:14 p.m., Vinod Kone wrote: > > src/tests/reconciliation_tests.cpp, line 137 > > <https://reviews.apache.org/r/48250/diff/1/?file=1408801#file1408801line137> > > > > state is a required field though. so not settting it is a bit weird. i > > would rather set a dummy state and add a comment saying it's dummy. > > > > status.set_state(TASK_KILLED); // Dummy. > > > > here and everywhere else.
Ah, good point! I wonder if we should move towards making `state` an optional field? Requiring that clients provide a dummy value is pretty counter-intuitive. - Neil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48250/#review136474 ----------------------------------------------------------- On June 7, 2016, 1:40 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48250/ > ----------------------------------------------------------- > > (Updated June 7, 2016, 1:40 p.m.) > > > Review request for mesos, Benjamin Mahler and Vinod Kone. > > > Bugs: MESOS-5547 > https://issues.apache.org/jira/browse/MESOS-5547 > > > Repository: mesos > > > Description > ------- > > Task state reconciliation is documented to only look at the > `taskId` and `slaveId` fields of the TaskStatus that is sent to > the master; hence, setting the `state` field of TaskStatus is > redundant at best and misleading at worst. > > Along the way, make use of C++11 initializer lists. > > > Diffs > ----- > > src/tests/reconciliation_tests.cpp 092dff165097315cbebd20bc32fb9fcf5370d136 > > Diff: https://reviews.apache.org/r/48250/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Neil Conway > >
