You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Niklas Nielsen <ni...@qni.dk> on 2015/03/05 01:03:55 UTC

Re: Review Request 30339: Use flags.hooks.isSome() before calling hooks.

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



src/slave/slave.hpp
<https://reviews.apache.org/r/30339/#comment118916>

    Can you split this into it's own commit or document it in the review request description?



src/slave/slave.cpp
<https://reviews.apache.org/r/30339/#comment122235>

    Needs 2 space indent extra.



src/tests/hook_tests.cpp
<https://reviews.apache.org/r/30339/#comment122232>

    s/> >/>>/g



src/tests/hook_tests.cpp
<https://reviews.apache.org/r/30339/#comment122233>

    Where does this constant come from? Should we perhaps move this into CreateMasterFlags()?



src/tests/hook_tests.cpp
<https://reviews.apache.org/r/30339/#comment122234>

    Same here.


- Niklas Nielsen


On Jan. 27, 2015, 4:42 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30339/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2015, 4:42 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/master/master.cpp bda8fda9bc2e52ccc1d75e2541e4604989515e13 
>   src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
>   src/slave/slave.cpp fca83b3977b95ddda30f9830da10e124b5c605e6 
>   src/tests/hook_tests.cpp 44f73effdce2d03627215418007ccbc3263a0c52 
> 
> Diff: https://reviews.apache.org/r/30339/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


Re: Review Request 30339: Use flags.hooks.isSome() before calling hooks.

Posted by Niklas Nielsen <ni...@qni.dk>.

> On March 4, 2015, 4:03 p.m., Niklas Nielsen wrote:
> >

Do you want this in? If so, please update the review :)


- Niklas


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


On Jan. 27, 2015, 4:42 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30339/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2015, 4:42 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/master/master.cpp bda8fda9bc2e52ccc1d75e2541e4604989515e13 
>   src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
>   src/slave/slave.cpp fca83b3977b95ddda30f9830da10e124b5c605e6 
>   src/tests/hook_tests.cpp 44f73effdce2d03627215418007ccbc3263a0c52 
> 
> Diff: https://reviews.apache.org/r/30339/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


Re: Review Request 30339: Use flags.hooks.isSome() before calling hooks.

Posted by Niklas Nielsen <ni...@qni.dk>.

> On March 4, 2015, 4:03 p.m., Niklas Nielsen wrote:
> >
> 
> Niklas Nielsen wrote:
>     Do you want this in? If so, please update the review :)

Ping - want this in? :)


- Niklas


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


On Jan. 27, 2015, 4:42 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30339/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2015, 4:42 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/master/master.cpp bda8fda9bc2e52ccc1d75e2541e4604989515e13 
>   src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
>   src/slave/slave.cpp fca83b3977b95ddda30f9830da10e124b5c605e6 
>   src/tests/hook_tests.cpp 44f73effdce2d03627215418007ccbc3263a0c52 
> 
> Diff: https://reviews.apache.org/r/30339/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


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

Posted by Kapil Arya <ka...@mesosphere.io>.

> On March 4, 2015, 7:03 p.m., Niklas Nielsen wrote:
> > src/tests/hook_tests.cpp, line 229
> > <https://reviews.apache.org/r/30339/diff/1/?file=837755#file837755line229>
> >
> >     Where does this constant come from? Should we perhaps move this into CreateMasterFlags()?

This is required for hooks only. The const is defined above in the file. It corresponds to the name of the module.


- Kapil


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


On June 1, 2015, 5:26 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:26 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
> 
>