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



include/mesos/mesos.hpp
<https://reviews.apache.org/r/29599/#comment111346>

    It is unintuitive where the definitions of these methods exist (currently 
src/common/type_utils.cpp). I would suggest to put these in 
include/type_utils.hpp to atleast give a hint to the user about the file name 
to look far. Admittedly, it is still not that intuitive, but is atleast in line 
with what we did with include/values.hpp and src/common/values.cpp.
    
    Also, instead of splitting the definitions of the operators across 
type_utils.hpp and .cpp, I would suggest to move all definitions to cpp, again 
similar to values.cpp. (do this in a dependent patch first).


- Vinod Kone


On Jan. 6, 2015, 10:39 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29599/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2015, 10:39 p.m.)
> 
> 
> Review request for mesos, Niklas Nielsen, Till Toenshoff, and Timothy Chen.
> 
> 
> Bugs: MESOS-2096
>     https://issues.apache.org/jira/browse/MESOS-2096
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> [2/9] Build Mesos Modules outside Mesos source tree.
> 
> These operators correspond to the protobufs declared in mesos.proto.  Since 
> mesos.hpp (that includes mesos.pb.h) exposes these protobus, these 
> definitions also need to be exposed.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.hpp 371d14a6cf802d1099be84f217a1af6554cc4eee 
>   src/common/type_utils.hpp 2d22db85f87ec5c1134208aae5b2105d2d267000 
>   src/common/type_utils.cpp fcc9eb0e27a9cbf53f7923f0c051a6578527ec37 
>   src/slave/state.hpp 592182b21ae2700c627f180db38ca5dc1b3c9e8a 
>   src/tests/registrar_tests.cpp 6bfb679d6ab07143f9c77b8cd9719a36d363645f 
> 
> Diff: https://reviews.apache.org/r/29599/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Reply via email to