----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40036/#review105513 -----------------------------------------------------------
Ship it! Thanks for the test! src/common/protobuf_utils.cpp (lines 178 - 187) <https://reviews.apache.org/r/40036/#comment164130> You don't need the 'if' with the way you've structured your loop, I assume it's just here from copy paste? src/common/protobuf_utils.cpp (line 182) <https://reviews.apache.org/r/40036/#comment164131> Consider: ``` for (auto status = task.statuses().rbegin(); status != task.statuses().rend(); ++status) ``` Ideally we could do: ``` foreach (const TaskStatus& status, reverse(task.statuses())) ``` If you want to avoid the indexing math. src/common/protobuf_utils.cpp (line 190) <https://reviews.apache.org/r/40036/#comment164129> two newlines here :) src/tests/master_tests.cpp (line 3369) <https://reviews.apache.org/r/40036/#comment164134> The convention here would be `status2` or something like `reconciliationStatus`. src/tests/master_tests.cpp (lines 3373 - 3377) <https://reviews.apache.org/r/40036/#comment164136> You can avoid the vector here by passing in an initializer list into your reconcileTasks call. src/tests/master_tests.cpp (line 3386) <https://reviews.apache.org/r/40036/#comment164137> We now have the `->` operator for Future, could you do a sweep in the code you've introduced to replace `.get().` with `->`? Thanks!! src/tests/master_tests.cpp (line 3394) <https://reviews.apache.org/r/40036/#comment164135> Ditto here. - Ben Mahler On Nov. 6, 2015, 7:53 p.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40036/ > ----------------------------------------------------------- > > (Updated Nov. 6, 2015, 7:53 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-3845 > https://issues.apache.org/jira/browse/MESOS-3845 > > > Repository: mesos > > > Description > ------- > > Along the lines of health checks, the container_status should also be sent > inside reconciliation updates. > > > Diffs > ----- > > src/common/protobuf_utils.hpp 44a2b1d3bab227294f5c5854acd87b7bd75ef589 > src/common/protobuf_utils.cpp 1e795dcc90b2225ad7b0a124dbdcdabdc9d2e6aa > src/master/master.cpp cbae27e7a4059a72bc69e152ec8adaf4ef725965 > src/tests/master_tests.cpp 856440559de7d58be5bcf2ab6be911b5c67001cd > > Diff: https://reviews.apache.org/r/40036/diff/ > > > Testing > ------- > > `make check` with updated `MasterTest.TaskStatusContainerStatus` test. > > > Thanks, > > Kapil Arya > >
