> On Feb. 22, 2015, 4:48 p.m., Kapil Arya wrote: > > src/slave/slave.cpp, line 1111 > > <https://reviews.apache.org/r/31016/diff/1/?file=863935#file863935line1111> > > > > Can we instead swap its name with `task_` and move the label decorator > > higher (right after entry into this function)? > > > > This would prevent future developers from accidentally using `task` > > when they meant to use `task_`. > > > > Doing the label decoration before the error checks has an added benefit > > of generating notifications for dropped tasks as well for the hook modules. > > Niklas Nielsen wrote: > I don't think we should reveal task infos that are failing. How should > the module implementer know the difference between a successfully launched > task and a failed one? > > The pattern is to append '_' as f(), f'(), f''(). So would keep that for > consistency.
I inlined the update so I could get rid of the helper (and copy) > On Feb. 22, 2015, 4:48 p.m., Kapil Arya wrote: > > src/slave/slave.cpp, lines 3924-3934 > > <https://reviews.apache.org/r/31016/diff/1/?file=863935#file863935line3924> > > > > Should we rather move it close to the runTask definition? Since the two > > are logically close. > > Niklas Nielsen wrote: > The ordering is slightly inconsistent (should follow the one from the > header file). > > I'd prefer not to cludder the code and methods around runTask() with > helpers - there is a lot of central pieces there already. Are you feeling > strongly about moving this up? Helper should be gone in the latest patch - Niklas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31016/#review73457 ----------------------------------------------------------- On April 7, 2015, 5:57 p.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31016/ > ----------------------------------------------------------- > > (Updated April 7, 2015, 5:57 p.m.) > > > Review request for mesos, Ben Mahler and Kapil Arya. > > > Repository: mesos > > > Description > ------- > > Added decorator which gets invoked on start of runTask() sequence in the > slave. > > > Diffs > ----- > > include/mesos/hook.hpp 9ae8b9455a86c7a5cbf4f1d1b1ce88f2811ce35d > src/hook/manager.hpp da813492108974a7e26b366845368517589da876 > src/hook/manager.cpp 7a4cb09bc221af502e867cfb7fff2900b599ff1f > src/slave/slave.hpp 19e6b44bc344c0ca509674803f401cbb4e1f47ae > src/slave/slave.cpp 9fec023b643d410f4d511fa6f80e9835bab95b7e > > Diff: https://reviews.apache.org/r/31016/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Niklas Nielsen > >
