mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 50492: Moved Network declaration into Mesos headers.
Date Mon, 01 Aug 2016 23:49:51 GMT

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




include/mesos/log/network.hpp (lines 31 - 34)
<https://reviews.apache.org/r/50492/#comment210424>

    Not yours, but can you take this chance to convert these comments to `/* ... */` format?
    
    We generally want public headers to take this form.



include/mesos/log/network.hpp (line 72)
<https://reviews.apache.org/r/50492/#comment210428>

    Not yours, but can you fix up the spacing here (`> > >`)?



src/Makefile.am (lines 558 - 560)
<https://reviews.apache.org/r/50492/#comment210423>

    I wish this folder didn't already exist.  If the `network` was the only file in this folder,
I would want it named `replicated_log/network.hpp`.
    
    "log" is an easily conflated term :(



src/log/network.hpp (lines 20 - 21)
<https://reviews.apache.org/r/50492/#comment210426>

    After considering the contents of `network.hpp/cpp`, there's actually nothing strictly
specific to the replicated log here.  So if we're going to move most of the code anyway, we
should move the general bits into libprocess.  (This shouldn't be too drastic of a change.)
    
    The end result should be files like:
    ```
    3rdparty/libprocess/include/process/network.hpp
    3rdparty/libprocess/src/network.cpp
    
    src/log/network/zookeeper.hpp
    src/log/network/zookeeper.cpp
    ```
    
    You may also consider renaming "network" to "pid_group".  "network" is another easily
conflated term :)


- Joseph Wu


On July 27, 2016, 10:23 p.m., Jay Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50492/
> -----------------------------------------------------------
> 
> (Updated July 27, 2016, 10:23 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5828
>     https://issues.apache.org/jira/browse/MESOS-5828
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Moved log Network declaration into include/mesos/log/network.hpp.
> This header is needed to compile log network module.
> 
> 
> Diffs
> -----
> 
>   include/mesos/log/network.hpp PRE-CREATION 
>   src/Makefile.am 3d33ee722543c67d0aa72e1c72c578066832331a 
>   src/log/log.cpp f8e439fc756af05acb40878a9473c93c039c889e 
>   src/log/network.hpp 8750880e6cecb6edf4efab4f2b9afe3d8a4d5964 
>   src/log/network.cpp PRE-CREATION 
>   src/log/zookeeper/network.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50492/diff/
> 
> 
> Testing
> -------
> 
> see the end of review chain
> 
> 
> Thanks,
> 
> Jay Guo
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message