Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-22 Thread Michael Park
> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, line 328 > > > > > > I don't like the name 'execute'. When you create the Subprocess > > instance, the

Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-20 Thread Jie Yu
> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, lines 119-130 > > > > > > Why not just use a single `int status` field here. The users can use > > WEXITSTATUS

Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-20 Thread Marco Massenzio
> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, lines 53-75 > > > > > > What's the motivation of storing this? Should the caller already have > > those

Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-20 Thread Jie Yu
> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, line 328 > > > > > > I don't like the name 'execute'. When you create the Subprocess > > instance, the

Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-20 Thread Jie Yu
> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, line 328 > > > > > > I don't like the name 'execute'. When you create the Subprocess > > instance, the

Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-20 Thread Jie Yu
> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, lines 53-75 > > > > > > What's the motivation of storing this? Should the caller already have > > those

Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-20 Thread Jie Yu
> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, lines 336-343 > > > > > > Why introduce this method? I think the caller should be responsible for > > killing

Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-20 Thread Marco Massenzio
> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, lines 53-75 > > > > > > What's the motivation of storing this? Should the caller already have > > those

Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-12 Thread Michael Park
> On Nov. 7, 2015, 1:06 a.m., Michael Park wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, line 340 > > > > > > I think this violates our use of `auto` rule. Could you please spell > > out the type?

Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-10 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37336/#review105960 --- Patch looks great! Reviews applied: [37336] All tests passed. -

Re: Review Request 37336: Added `execute()` method to process::Subprocess

2015-11-10 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37336/ --- (Updated Nov. 10, 2015, 8:51 p.m.) Review request for mesos, Joris Van