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


Looks great! A few more cleanups and then we'll get this committed.


3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp
<https://reviews.apache.org/r/19093/#comment68572>

    Please add a newline between these two (that's the style in the code base).



3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp
<https://reviews.apache.org/r/19093/#comment68573>

    We don't indent anything within namespaces in our codebase, please pull 
this back -2.



3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp
<https://reviews.apache.org/r/19093/#comment68569>

    You've got an extra space on this line too.



3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp
<https://reviews.apache.org/r/19093/#comment68570>

    One of these things is not like the other. ;) I'd like to make two 
suggestions:
    
    (1) Replace 'readable', 'writeable', and 'executable' with 'r', 'w', and 
'x' respectively and keep 'rwx'.
    
    (2) Replace 'rwx' with 'all' (or another name you find more natural).
    
    Even though (1) is a bit more esoteric I still think it's easier to grok 
than S_IRUSR, S_IWUSR, etc. and it's fairly reminiscent of using 'chmod' on the 
command line.
    
    What do you think?



src/master/master.cpp
<https://reviews.apache.org/r/19093/#comment68571>

    Please add a newline between these two: there's a lot of text here and it's 
easier to read when they're separated.


- Benjamin Hindman


On March 13, 2014, 8:47 a.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19093/
> -----------------------------------------------------------
> 
> (Updated March 13, 2014, 8:47 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
> 
> 
> Bugs: MESOS-1087
>     https://issues.apache.org/jira/browse/MESOS-1087
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Adding permissions check on credentials file and deleting TODO comment
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 1075b46 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 3f475a4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp 
> PRE-CREATION 
>   src/master/master.cpp 2a40333 
> 
> Diff: https://reviews.apache.org/r/19093/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Reply via email to