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




src/module/manager.hpp (line 24)
<https://reviews.apache.org/r/45419/#comment188730>

    Not yours, but doesn't seem to be used.



src/module/manager.hpp (line 33)
<https://reviews.apache.org/r/45419/#comment188732>

    Not yours, but doesn't seem to be used.



src/module/manager.hpp (lines 34 - 36)
<https://reviews.apache.org/r/45419/#comment188731>

    Not yours, but could you update this with the complete include list? I see 
use of definitions from at least `stout/errorbase.hpp`, `stout/foreach.hpp`, 
`stout/none.hpp`, `stout/option.hpp` and `stout/try.hpp`.



src/module/manager.cpp (line 27)
<https://reviews.apache.org/r/45419/#comment188733>

    We don't seem to include headers both in the header and implementation file.



src/module/manager.cpp (lines 28 - 29)
<https://reviews.apache.org/r/45419/#comment188735>

    Not yours, but doesn't seem to be used.



src/module/manager.cpp (lines 31 - 32)
<https://reviews.apache.org/r/45419/#comment188724>

    Not yours, but could you sort these alphabetically?



src/module/manager.cpp (line 32)
<https://reviews.apache.org/r/45419/#comment188736>

    Not yours, but doesn't seem to be used.



src/module/manager.cpp (line 34)
<https://reviews.apache.org/r/45419/#comment188737>

    Could you makes these complete? I see the implementation is using 
definitions from at least `stout/check.hpp`, `stout/nothing.hpp`, 
`stout/synchronized.hpp`, and `process/owned.hpp`.



src/module/manager.cpp 
<https://reviews.apache.org/r/45419/#comment188725>

    Usually we separate declarations outside of classes etc. by two empty 
lines, so I think this line should not have been removed. Note that there is 
some inconsitency here, since strictly following this rule would also make use 
set the declarations of above globals apart by two empty lines.


- Benjamin Bannier


On March 29, 2016, 11:17 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45419/
> -----------------------------------------------------------
> 
> (Updated March 29, 2016, 11:17 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removes unused includes and some unsused usings from the ModuleManager.
> 
> 
> Diffs
> -----
> 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 8c9aaf7cd00c904daba9994a99df9e1329831c01 
> 
> Diff: https://reviews.apache.org/r/45419/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Reply via email to