mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joris Van Remoortere" <joris.van.remoort...@gmail.com>
Subject Re: Review Request 29288: stout: Created IP address abstraction for different protocol families
Date Tue, 27 Jan 2015 23:54:02 GMT


> On Jan. 27, 2015, 6:32 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 259
> > <https://reviews.apache.org/r/29288/diff/9-10/?file=834346#file834346line259>
> >
> >     I think the pattern here is 2 spaces, colon, first initializer, then 4 spaces
on the next rows. This is to align all of the member variables vertically like so:
> >     `
> >     explicit MyClass(int64_t _i64, int32_t _i32)
> >       : i64(_i64)
> >         i32(_i32)
> >     `
> >     Can you double check that please? :-)
> 
> Evelina Dumitrescu wrote:
>     Btw, I found an inconsistency regarding initializing variables in the initializing
list for the constructors. Here are several patterns:
>     - 2 space and the list of variables on a single line 
>       * pid.hpp 
>     - 2 spaces and the memebers fo the list on a different lines
>       * event.hpp for DispatchEvent struct
>       * gmock.h  for FutureResultAction class
>       *struct Response the list of parameters on the next line, even though it would
have fit on the previous line

Yeah the codebase is inconsistent. and apparently my code block didn't accept the new lines
which added confusion. Sorry about this.
```C++
explicit MyClass(int64_t _i64, int32_t _i32)
  : i64(_i64),
    i32(_i32)
    ```


> On Jan. 27, 2015, 6:32 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, lines 602-604
> > <https://reviews.apache.org/r/29288/diff/9-10/?file=834346#file834346line602>
> >
> >     We tend to indent these like this:
> >     `
> >     inline Try<IP> IP::fromAddressNetmask(
> >         const IP::InternetAddress& address,
> >         const IP::InternetAddress& netmask)
> >     `
> >     
> >     Here and below please.
> 
> Evelina Dumitrescu wrote:
>     the line is over 80 charachters, how should I split it ?

Sorry, my code blocks didn't come out right:
```C++
inline Try<IP> IP::fromAddressNetmask(
    const IP::InternetAddress& address, 
    const IP::InternetAddress& netmask)
{
  ...
}
```


- Joris


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


On Jan. 27, 2015, 11:05 p.m., Evelina Dumitrescu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29288/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2015, 11:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van Remoortere,
and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1919
>     https://issues.apache.org/jira/browse/MESOS-1919
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Created the inner class InAddrStorage encapsulated inside the IP class.
> The class uses a union with the in_addr and in6_addr fields.
> I considered that the The MasterInfo protobuffers should have both an ipv4 and an ipv6
field.
> I intend to use the same Classifiers, addition, removal and update of container filters,
but write different encode/decode functions for IPv4/ICMP and IPv6/ICMPv6 because the processing
of the protocol headers differ.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp a0210ea6440086246aafe632f86498abbb70719a

>   3rdparty/libprocess/3rdparty/stout/tests/net_tests.cpp 425132e5d7c3770be4a5a39feea5a2f22179b871

> 
> Diff: https://reviews.apache.org/r/29288/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Evelina Dumitrescu
> 
>


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