----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28656/#review66814 -----------------------------------------------------------
High-level comments from the previous RR also applies here (abstract class and Try return type). Another one is the break up of launch task, I'd like to get BenH or BenM's take on it. src/hook/hook.hpp <https://reviews.apache.org/r/28656/#comment110472> From previous RR: Shouldn't we make the return type a Try, so we can ignore the result from failed hooks? src/slave/flags.hpp <https://reviews.apache.org/r/28656/#comment110450> Can you isolate fly-fixes in another review? Here and below src/slave/slave.cpp <https://reviews.apache.org/r/28656/#comment110453> s/newEnvironment/environment/g You only have one. src/slave/slave.cpp <https://reviews.apache.org/r/28656/#comment110474> Can you find an example where we wrap like this? To distinguish that it is the environment pointer you are evnentually merging, I tend to lean toward either splitting this up (Get the command pointer and then the environment pointer) or just nest with 2 additional spaces for environment and make it on one line. src/slave/slave.cpp <https://reviews.apache.org/r/28656/#comment110473> Why this refactor? Couldn't you just inline the hook like in the master? - Niklas Nielsen On Jan. 5, 2015, 2:42 p.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28656/ > ----------------------------------------------------------- > > (Updated Jan. 5, 2015, 2:42 p.m.) > > > Review request for mesos and Niklas Nielsen. > > > Bugs: MESOS-2060 > https://issues.apache.org/jira/browse/MESOS-2060 > > > Repository: mesos-git > > > Description > ------- > > Similar to label decorator for Master. > > > Diffs > ----- > > src/hook/hook.hpp PRE-CREATION > src/hook/manager.hpp PRE-CREATION > src/hook/manager.cpp PRE-CREATION > src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef > src/slave/main.cpp 2ff2b0d186dd63e1437956e98e3682d83f2f5cd6 > src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 > src/slave/slave.cpp 50b57819b55bdcdb9f49f20648199badc4d3f37b > > Diff: https://reviews.apache.org/r/28656/diff/ > > > Testing > ------- > > > Thanks, > > Kapil Arya > >
