----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31028/#review79152 -----------------------------------------------------------
src/examples/test_hook_module.cpp <https://reviews.apache.org/r/31028/#comment128291> The foreach `label` shadows the `label` from line 79. And `label_` from line 86 doesn't need to be a different variable from the other `Label*` on line 79. Maybe oldLabel/newLabel are appropriate? src/tests/hook_tests.cpp <https://reviews.apache.org/r/31028/#comment128292> No reason to explicitly CreateMasterFlags if you're not reusing `masterFlags` elsewhere or trying to force a new tmpDir. src/tests/hook_tests.cpp <https://reviews.apache.org/r/31028/#comment128293> Unused. src/tests/hook_tests.cpp <https://reviews.apache.org/r/31028/#comment128295> CopyFrom src/tests/hook_tests.cpp <https://reviews.apache.org/r/31028/#comment128296> s/remove/removed/ src/tests/hook_tests.cpp <https://reviews.apache.org/r/31028/#comment128297> One which will... and the other...? Unclear what the second label is for (testing add) - Adam B On March 13, 2015, 4:04 p.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31028/ > ----------------------------------------------------------- > > (Updated March 13, 2015, 4:04 p.m.) > > > Review request for mesos, Ben Mahler and Kapil Arya. > > > Repository: mesos > > > Description > ------- > > See summary > > > Diffs > ----- > > src/examples/test_hook_module.cpp 47409cd4d02e238d1d182571d92019114662cd41 > src/tests/hook_tests.cpp bb9de25bd2c4601d333a3ca1aec13820c7df7378 > > Diff: https://reviews.apache.org/r/31028/diff/ > > > Testing > ------- > > make check (with newly added VerifySlaveRunTaskHook test) > > > Thanks, > > Niklas Nielsen > >
