----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23698/#review48950 -----------------------------------------------------------
Looks great, Craig. Just a few style/naming/comment nits, and a request to change NE(0) to EQ(1) to be more specific. Clean these up and I can get this committed for you. src/tests/fault_tolerance_tests.cpp <https://reviews.apache.org/r/23698/#comment85771> Alphabetical order please. src/tests/fault_tolerance_tests.cpp <https://reviews.apache.org/r/23698/#comment85773> Use double-line spacing between functions at the top-level. src/tests/fault_tolerance_tests.cpp <https://reviews.apache.org/r/23698/#comment85775> Maybe do the parsing after verifying the header is json (below). src/tests/fault_tolerance_tests.cpp <https://reviews.apache.org/r/23698/#comment85789> What is a "JObj"? Makes me think of Java Perhaps masterJSON or masterStateJSON would be clearer? src/tests/fault_tolerance_tests.cpp <https://reviews.apache.org/r/23698/#comment85792> Maybe even EXPECT_EQ(1u)? src/tests/fault_tolerance_tests.cpp <https://reviews.apache.org/r/23698/#comment85790> Again, JObj is hard to read. src/tests/fault_tolerance_tests.cpp <https://reviews.apache.org/r/23698/#comment85793> Let's add a comment above this to explain why we expect the framework to still be "running" after killing the task. (Because the executor has to timeout before it will exit/complete.) src/tests/fault_tolerance_tests.cpp <https://reviews.apache.org/r/23698/#comment85794> Remove blank line. - Adam B On July 25, 2014, 7:11 p.m., Craig Hansen-Sturm wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23698/ > ----------------------------------------------------------- > > (Updated July 25, 2014, 7:11 p.m.) > > > Review request for mesos and Adam B. > > > Bugs: MESOS-1626 > https://issues.apache.org/jira/browse/MESOS-1626 > > > Repository: mesos-git > > > Description > ------- > > Addresses a TODO workitem in ReregisterCompletedFrameworks. Manually parsed > JSON for master and slave state now uses stout JSON utility classes. > > > Diffs > ----- > > src/tests/fault_tolerance_tests.cpp ecf7d5e > > Diff: https://reviews.apache.org/r/23698/diff/ > > > Testing > ------- > > Make check. > > > Thanks, > > Craig Hansen-Sturm > >
