Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-17 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/ --- (Updated June 17, 2015, 5:06 p.m.) Review request for mesos, Niklas Nielsen and

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/#review88088 --- Ship it! Ship It! - Niklas Nielsen On June 15, 2015, 3:26 p.m.,

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-16 Thread Joris Van Remoortere
> On June 16, 2015, 4:55 p.m., Niklas Nielsen wrote: > > src/hook/manager.cpp, lines 96-98 > > > > > > Won't some compilers potentially break with reaching end of non-void > > function? > > > > How about: > >

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/#review88085 --- src/hook/manager.cpp (lines 96 - 98)

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/#review87991 --- Patch looks great! Reviews applied: [30339] All tests passed. - M

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-15 Thread Kapil Arya
> On June 4, 2015, 3:39 a.m., Till Toenshoff wrote: > > src/tests/hook_tests.cpp, line 132 > > > > > > Could you please explain quickly on why this was ever working / is > > needed now? Good point. As it turns out, w

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-15 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/ --- (Updated June 15, 2015, 6:26 p.m.) Review request for mesos, Niklas Nielsen and

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-15 Thread Kapil Arya
> On June 3, 2015, 1:17 p.m., Niklas Nielsen wrote: > > src/hook/manager.cpp, line 95 > > > > > > Don't you need to acquire the mutex here? Good catch. Fixed. - Kapil ---

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-15 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/ --- (Updated June 15, 2015, 5:22 p.m.) Review request for mesos, Niklas Nielsen and

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-04 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/#review86574 --- src/hook/manager.cpp

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-03 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/#review86436 --- src/hook/manager.cpp

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-02 Thread Niklas Nielsen
> On June 1, 2015, 2:41 p.m., Niklas Nielsen wrote: > > Do you have a JIRA ticket for this change? > > > > Is it to avoid memory copies, that you need these to be in the call sites. > > It clutters the call sites a bit, so I wanted to seee if we could move this > > into the HooksManager instea

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/#review86103 --- Patch looks great! Reviews applied: [30339] All tests passed. - M

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-01 Thread Kapil Arya
> On June 1, 2015, 5:41 p.m., Niklas Nielsen wrote: > > Do you have a JIRA ticket for this change? > > > > Is it to avoid memory copies, that you need these to be in the call sites. > > It clutters the call sites a bit, so I wanted to seee if we could move this > > into the HooksManager instea

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-01 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/ --- (Updated June 1, 2015, 5:46 p.m.) Review request for mesos, Niklas Nielsen and

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-01 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/#review86091 --- Do you have a JIRA ticket for this change? Is it to avoid memory co

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-01 Thread Kapil Arya
> On March 4, 2015, 7:03 p.m., Niklas Nielsen wrote: > > src/tests/hook_tests.cpp, line 229 > > > > > > Where does this constant come from? Should we perhaps move this into > > CreateMasterFlags()? This is required f

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-01 Thread Kapil Arya
> On Feb. 18, 2015, 2:34 a.m., Till Toenshoff wrote: > > src/tests/hook_tests.cpp, line 239 > > > > > > Sharp bracket closing; that part somehow slipped through the recent > > "hooks" commits - there are more of those

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-01 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/ --- (Updated June 1, 2015, 5:26 p.m.) Review request for mesos, Niklas Nielsen and