Re: Review Request 46340: Stout:[1/2] Implement `os::waitpid`.

2016-04-21 Thread Joris Van Remoortere 

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




3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 254)


start with a capital.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 268 - 
269)


new line.


- Joris Van Remoortere


On April 20, 2016, 9:51 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46340/
> ---
> 
> (Updated April 20, 2016, 9:51 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
> 
>

Re: Review Request 46340: Stout:[1/2] Implement `os::waitpid`.

2016-04-20 Thread Alex Clemmer 

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

(Updated April 20, 2016, 9:51 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 (updated)
-

  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

Re: Review Request 46340: Stout:[1/2] Implement `os::waitpid`.

2016-04-20 Thread Joris Van Remoortere 

---
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)


spelling.



3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (line 409)


grammar



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 254)


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



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 261)


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)


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)


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)


`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)


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
> 
>

Review Request 46340: Stout:[1/2] Implement `os::waitpid`.

2016-04-18 Thread Alex Clemmer 

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

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