You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2015/06/03 15:45:17 UTC
Review Request 35000: Doxygen'ized Subprocess.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35000/
-----------------------------------------------------------
Review request for mesos, Bernd Mathiske and Joerg Schad.
Repository: mesos
Description
-------
See summary.
Diffs
-----
3rdparty/libprocess/include/process/subprocess.hpp 37cab7755d2890619b64e1ca09e0b7ad0e72cf76
Diff: https://reviews.apache.org/r/35000/diff/
Testing
-------
make check
doxygen ../Doxyfile
Thanks,
Benjamin Hindman
Re: Review Request 35000: Doxygen'ized Subprocess.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On June 4, 2015, 10:20 a.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 141
> > <https://reviews.apache.org/r/35000/diff/1/?file=977410#file977410line141>
> >
> > Suggestion: maybe we should add what structure this "status" has. It is propagated to here from an underlying call to waitpid()and associated macros from "wait.h" can be applied, as for example 'WIFEXITED(status)'.
Great idea!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35000/#review86577
-----------------------------------------------------------
On June 3, 2015, 1:45 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35000/
> -----------------------------------------------------------
>
> (Updated June 3, 2015, 1:45 p.m.)
>
>
> Review request for mesos, Bernd Mathiske and Joerg Schad.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 37cab7755d2890619b64e1ca09e0b7ad0e72cf76
>
> Diff: https://reviews.apache.org/r/35000/diff/
>
>
> Testing
> -------
>
> make check
> doxygen ../Doxyfile
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 35000: Doxygen'ized Subprocess.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35000/#review86577
-----------------------------------------------------------
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/35000/#comment138631>
Nit: Capitalized sentence start after ", and". This was there before, but now is a great opportunity to clean it up.
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/35000/#comment138632>
Suggestion: delete "Helper that". (Everywhere)
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/35000/#comment138634>
Suggestion: maybe we should add what structure this "status" has. It is propagated to here from an underlying call to waitpid()and associated macros from "wait.h" can be applied, as for example 'WIFEXITED(status)'.
- Bernd Mathiske
On June 3, 2015, 6:45 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35000/
> -----------------------------------------------------------
>
> (Updated June 3, 2015, 6:45 a.m.)
>
>
> Review request for mesos, Bernd Mathiske and Joerg Schad.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 37cab7755d2890619b64e1ca09e0b7ad0e72cf76
>
> Diff: https://reviews.apache.org/r/35000/diff/
>
>
> Testing
> -------
>
> make check
> doxygen ../Doxyfile
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 35000: Doxygen'ized Subprocess.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35000/#review86787
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/35000/#comment138867>
How about starting both enumerations with lower case here to make them part of the same sentence?
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/35000/#comment138868>
This leading sentence can exist without the colon and hence the following enumerations could use capitalized sentence starts.
I would replace the colon with a period to enable full sentences below.
- Till Toenshoff
On June 3, 2015, 1:45 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35000/
> -----------------------------------------------------------
>
> (Updated June 3, 2015, 1:45 p.m.)
>
>
> Review request for mesos, Bernd Mathiske and Joerg Schad.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 37cab7755d2890619b64e1ca09e0b7ad0e72cf76
>
> Diff: https://reviews.apache.org/r/35000/diff/
>
>
> Testing
> -------
>
> make check
> doxygen ../Doxyfile
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 35000: Doxygen'ized Subprocess.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35000/
-----------------------------------------------------------
(Updated June 14, 2015, 5:17 p.m.)
Review request for mesos, Bernd Mathiske and Joerg Schad.
Changes
-------
Updates from review comments.
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/include/process/subprocess.hpp 37cab7755d2890619b64e1ca09e0b7ad0e72cf76
Diff: https://reviews.apache.org/r/35000/diff/
Testing
-------
make check
doxygen ../Doxyfile
Thanks,
Benjamin Hindman
Re: Review Request 35000: Doxygen'ized Subprocess.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35000/#review86407
-----------------------------------------------------------
Patch looks great!
Reviews applied: [35000]
All tests passed.
- Mesos ReviewBot
On June 3, 2015, 1:45 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35000/
> -----------------------------------------------------------
>
> (Updated June 3, 2015, 1:45 p.m.)
>
>
> Review request for mesos, Bernd Mathiske and Joerg Schad.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 37cab7755d2890619b64e1ca09e0b7ad0e72cf76
>
> Diff: https://reviews.apache.org/r/35000/diff/
>
>
> Testing
> -------
>
> make check
> doxygen ../Doxyfile
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 35000: Doxygen'ized Subprocess.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35000/#review86404
-----------------------------------------------------------
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/35000/#comment138368>
s/returns/return
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/35000/#comment138369>
s/returns/return
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/35000/#comment138370>
s/returns/return
same for rest of file
Great effort, makes understanding Subprocess much simpler!
- Joerg Schad
On June 3, 2015, 1:45 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35000/
> -----------------------------------------------------------
>
> (Updated June 3, 2015, 1:45 p.m.)
>
>
> Review request for mesos, Bernd Mathiske and Joerg Schad.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 37cab7755d2890619b64e1ca09e0b7ad0e72cf76
>
> Diff: https://reviews.apache.org/r/35000/diff/
>
>
> Testing
> -------
>
> make check
> doxygen ../Doxyfile
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 35000: Doxygen'ized Subprocess.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35000/#review86423
-----------------------------------------------------------
Just a fly by comment: can't get my hands down for this initiative! Looking so much forward to browse and enhance our docs!
- Niklas Nielsen
On June 3, 2015, 6:45 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35000/
> -----------------------------------------------------------
>
> (Updated June 3, 2015, 6:45 a.m.)
>
>
> Review request for mesos, Bernd Mathiske and Joerg Schad.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 37cab7755d2890619b64e1ca09e0b7ad0e72cf76
>
> Diff: https://reviews.apache.org/r/35000/diff/
>
>
> Testing
> -------
>
> make check
> doxygen ../Doxyfile
>
>
> Thanks,
>
> Benjamin Hindman
>
>