> 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 instead.
> > 
> > Could we wrap the hook calls in a macro or a helper taking a lambda, which 
> > could simplify the call sites?

I don't think there is a JIRA ticket for this change. It was a suggestion by 
vinod or Ben.

Creating macros around it would be unclean. In certain cases, we are relying on 
the return value to update some fields and putting macros would just make it 
ugly. To me this current form seems more readable and makes it clear to the 
reader that the code would be called only if there are some hooks installed.


- Kapil


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30339/#review86091
-----------------------------------------------------------


On June 1, 2015, 5:46 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> 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 Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Call hook manager only if hooks were specified on the commandline.
> 
> 
> Diffs
> -----
> 
>   src/hook/manager.hpp 638e19f5062f070cd0ec94d830615a2c73fd357d 
>   src/hook/manager.cpp 54b0d34b6e9f2f8a8cf7a6c2f5ded2f6ab6c6955 
>   src/master/master.cpp 710b8149c9d855d0f47cb2952366be10bc78c74d 
>   src/slave/containerizer/containerizer.cpp 
> 4d66e767de1f877cb66b37826ba7c9d00639a7c0 
>   src/slave/slave.cpp 271cb03770cd08406054dfce35d0821475e49b05 
>   src/tests/hook_tests.cpp 3ffde6d6b2faeb5a8a40eb27c3b0a2b7f9ecd2b1 
> 
> Diff: https://reviews.apache.org/r/30339/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Reply via email to