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


Fix it, then Ship it!





3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp (line 128)
<https://reviews.apache.org/r/46340/#comment193312>

    spelling.



3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (line 409)
<https://reviews.apache.org/r/46340/#comment193309>

    grammar



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 254)
<https://reviews.apache.org/r/46340/#comment193323>

    Why the `In plain English:`?
    I think just the comment is sufficient.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 261)
<https://reviews.apache.org/r/46340/#comment193324>

    start the comment with a capital:
    `: Check ...`
    Missing some punctuations between `processes if not`?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 265 - 
267)
<https://reviews.apache.org/r/46340/#comment193326>

    Can you wrap the parameters one on each line?
    Also leave a new line after an expression that doesn't fit on a single line.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 269)
<https://reviews.apache.org/r/46340/#comment193328>

    Can we use `stringify` here? I think we let some of these slip previously. 
Can we have a cleanup review that fixes these with `stringify`?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 306 - 
308)
<https://reviews.apache.org/r/46340/#comment193330>

    `so return 0` is deceptive. Can we say something like `return `None` to map 
to `waitpid` return of 0`?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 330 - 
334)
<https://reviews.apache.org/r/46340/#comment193333>

    This must have slipped in a merge elsewhere. Can you cut this in to its own 
review?


- Joris Van Remoortere


On April 18, 2016, 6:52 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46340/
> -----------------------------------------------------------
> 
> (Updated April 18, 2016, 6:52 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4466
>     https://issues.apache.org/jira/browse/MESOS-4466
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Stout:[1/2] Implement `os::waitpid`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> a7f855dc9d0a87fe3b6d1611e7ae22e4d7cd7b6d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/46340/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Reply via email to