mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Hindman <b...@berkeley.edu>
Subject Re: Review Request 59688: Moved `net::IPNetwork` to `net::IP::Network`.
Date Fri, 02 Jun 2017 00:33:25 GMT

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



This needs to get split between Stout and Mesos please!


3rdparty/stout/include/stout/ip.hpp
Lines 208 (patched)
<https://reviews.apache.org/r/59688/#comment250144>

    Ideally we move this to net::inet::IP::Network::LOOPBACK(), but perhaps add a TODO for
now if this is not trivial.



3rdparty/stout/include/stout/ip.hpp
Lines 211 (patched)
<https://reviews.apache.org/r/59688/#comment250145>

    Same comment as above for `LOOPBACK_V4()`.



3rdparty/stout/include/stout/ip.hpp
Lines 251 (patched)
<https://reviews.apache.org/r/59688/#comment250147>

    Looks like the formatting got funky in this function when you moved, can you please fix?
Thanks!



3rdparty/stout/include/stout/ip.hpp
Lines 291-292 (patched)
<https://reviews.apache.org/r/59688/#comment250148>

    It would be great to have a comment here explaining why you need to use `std::unique_ptr`
so others don't have to try and read your mind! ;-) I don't even always remember the C++ rules
here.
    
    Another thought is to use a `std::shared_ptr` here instead. The advantage being that maybe
then you wouldn't need the extra copy constructor? This class is constant, i.e., you never
manipulate `address_` or `netmask_`, but actually making this `std::shared_ptr<const IP>`
might be just as tricky. Either way, comment on why this has to be a pointer!


- Benjamin Hindman


On May 31, 2017, 6:37 p.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59688/
> -----------------------------------------------------------
> 
> (Updated May 31, 2017, 6:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
>     https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Moved `net::IPNetwork` to `net::IP::Network`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/ip.hpp ad2bd922158eecd12b51b272d2a003b8ec8d3550 
>   3rdparty/stout/tests/ip_tests.cpp 4c6f81bd53413360182b447ddb149a18e406870e 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 3aecd8c5a2eb1319fe31ebeba815b6766e50904f

>   src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
ce795628f19f0e8138ad9a0b50b7d644b9d734a8 
>   src/tests/containerizer/cni_isolator_tests.cpp 565e58ae75e918453e4386f5e35a5a844a8b15f8

>   src/tests/utils.hpp a2ae43c6a20ab3753a3501d8ce23ffb0f6ac0005 
>   src/tests/utils.cpp 053976d3c4cf120ff5e7dff6e96c6862a5b617b1 
> 
> 
> Diff: https://reviews.apache.org/r/59688/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


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