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


Looks very clean, thanks Nikita!

(We should note that we use 'issues' in ReviewBoard as something we would like 
to track as fixed or dropped, these could even be style nits as you saw above. 
I really wish they had something like a 'task' for this, but oh well :)).

The main thing is it would be nice if we made process::socket() 'inline' in 
socket.hpp (sorry I didn't mention that before!).


3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/18001/#comment64373>

    This could go in the socket.hpp header if declared inline, sorry about that!
    
    Could you add a newline above and below the ifdef?



3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/18001/#comment64374>

    Can you move the declaration down here?
    
    int s = sock.get();
    
    Alternatively, we can just s/s/sock.get()/ in the code, but up to you.
    
    Also, is it problematic to name it s/sock/socket/?
    
    Ditto in the other cases in the diff.



3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/18001/#comment64375>

    Ditto here for moving the 'int' declaration down and s/sock/socket/ if 
possible.


- Ben Mahler


On Feb. 12, 2014, 9 p.m., Nikita Vetoshkin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18001/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2014, 9 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-912
>     https://issues.apache.org/jira/browse/MESOS-912
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Following the discussion in JIRA - disable SIGPIPE on MacOS via setsockopt as 
> it doesn't support MSG_NOSIGNAL.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp 
> 2e7764a8e0badec704b8610f3f72f0bd16cc9612 
> 
> Diff: https://reviews.apache.org/r/18001/diff/
> 
> 
> Testing
> -------
> 
> None.
> 
> 
> Thanks,
> 
> Nikita Vetoshkin
> 
>

Reply via email to