> On Dec. 6, 2014, 1:55 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/socket.hpp, line 40
> > <https://reviews.apache.org/r/28716/diff/7/?file=783988#file783988line40>
> >
> >     Hm.. this comment is a bit odd, because it's on top of the first 
> > method, but not on top of the others.
> >     
> >     Mind removing it, or adding something more standalone (i.e. newline):
> >     
> >     ```
> >     // Socket call wrappers for different address families.
> >     
> >     inline ...
> >     ```

Thank you for pointing me that out! I submitted another review for this  
https://reviews.apache.org/r/28789/ .


> On Dec. 6, 2014, 1:55 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/socket.hpp, line 50
> > <https://reviews.apache.org/r/28716/diff/7/?file=783988#file783988line50>
> >
> >     What does 'rc' mean? :)
> >     
> >     Please just call it 'accept' or 'result', it's ok to hide the name in 
> > the former case.

Sorry for that. I was used with this convention from other projects(eg: linux 
kernel). It's the abreviation from return code. dhamon submitted a fix for that 
https://github.com/apache/mesos/commit/83d90df125f05ab3c4f1b31311f7e5f1c9d6f8c1.


> On Dec. 6, 2014, 1:55 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/socket.hpp, lines 51-52
> > <https://reviews.apache.org/r/28716/diff/7/?file=783988#file783988line51>
> >
> >     Looks like the lack of braces here was missed.

See dhamon's last commit.


> On Dec. 6, 2014, 1:55 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/http.cpp, line 85
> > <https://reviews.apache.org/r/28716/diff/7/?file=783989#file783989line85>
> >
> >     The pattern we use for succinctness and clarity in naming here is:
> >     
> >     ```
> >     Try<int> connect = process::connect(...);
> >     ```
> >     
> >     "try" is redundant here since it's encoded in the type.
> >     
> >     Ditto everywhere else. :)

See dhamon's last commit.


- Evelina


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


On Dec. 5, 2014, 9:21 p.m., Evelina Dumitrescu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28716/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2014, 9:21 p.m.)
> 
> 
> Review request for mesos and Dominic Hamon.
> 
> 
> Bugs: MESOS-2177
>     https://issues.apache.org/jira/browse/MESOS-2177
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Created accept, bind, connect and getsockname wrappers in socket.hpp for 
> different protocol families
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> ab080c154095029c4a01d189a5fd8a178ba6c92e 
>   3rdparty/libprocess/src/http.cpp b00f33339366f5c06b6f20e38c5ae0c23b8a9358 
>   3rdparty/libprocess/src/process.cpp 
> 4db7d56af710577e08a9d7dbeb92a1f01559401f 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> a90e65f77904da0a45e1cc0cc9889ae69354a1a5 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> dec62e88ec993433e1a0777593bb2657b43636dc 
> 
> Diff: https://reviews.apache.org/r/28716/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Evelina Dumitrescu
> 
>

Reply via email to