Re: Review Request 42662: Added common command utils file.

2016-01-27 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42662/#review116665 --- src/common/command_utils.cpp (line 17)

Re: Review Request 42662: Added common command utils file.

2016-01-27 Thread Jie Yu
> On Jan. 28, 2016, 12:52 a.m., Klaus Ma wrote: > > src/common/command_utils.cpp, line 17 > > > > > > Move to line 28. > > Jojy Varghese wrote: > According to google style guide >

Re: Review Request 42662: Added common command utils file.

2016-01-27 Thread Jojy Varghese
> On Jan. 28, 2016, 12:52 a.m., Klaus Ma wrote: > > src/tests/common/command_utils_tests.cpp, line 71 > > > > > > Add a case for `tar/untar` failure. > > Jojy Varghese wrote: > Will add a TODO. > > Jie Yu

Re: Review Request 42662: Added common command utils file.

2016-01-27 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42662/ --- (Updated Jan. 28, 2016, 2:21 a.m.) Review request for mesos and Jie Yu.

Re: Review Request 42662: Added common command utils file.

2016-01-27 Thread Jojy Varghese
> On Jan. 28, 2016, 12:52 a.m., Klaus Ma wrote: > > src/common/command_utils.cpp, line 17 > > > > > > Move to line 28. According to google style guide

Re: Review Request 42662: Added common command utils file.

2016-01-27 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42662/#review116600 --- Fix it, then Ship it! This is looking good! Some nits and

Re: Review Request 42662: Added common command utils file.

2016-01-27 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42662/ --- (Updated Jan. 27, 2016, 8:29 p.m.) Review request for mesos and Jie Yu.

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42662/#review116116 --- Haven't looked at the tests yet. Will do another pass once the

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jojy Varghese
> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote: > > src/common/command_utils.cpp, line 46 > > > > > > We try to avoid using Option> since the semantics is not > > clear between None() and an empty vector. >

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jojy Varghese
> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote: > > src/common/command_utils.cpp, line 46 > > > > > > We try to avoid using Option> since the semantics is not > > clear between None() and an empty vector. >

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jojy Varghese
> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote: > > src/common/command_utils.cpp, line 46 > > > > > > We try to avoid using Option> since the semantics is not > > clear between None() and an empty vector. >

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jojy Varghese
> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote: > > src/common/command_utils.cpp, lines 153-162 > > > > > > I don't think this check is necessary. We are basically checking what > > 'tar' will check later. Can you

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jie Yu
> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote: > > src/common/command_utils.cpp, lines 153-162 > > > > > > I don't think this check is necessary. We are basically checking what > > 'tar' will check later. Can you

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jojy Varghese
> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote: > > src/common/command_utils.cpp, line 46 > > > > > > We try to avoid using Option> since the semantics is not > > clear between None() and an empty vector. >

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jie Yu
> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote: > > src/common/command_utils.cpp, line 46 > > > > > > We try to avoid using Option> since the semantics is not > > clear between None() and an empty vector. >

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jojy Varghese
> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote: > > src/common/command_utils.cpp, line 46 > > > > > > We try to avoid using Option> since the semantics is not > > clear between None() and an empty vector. >

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jie Yu
> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote: > > src/common/command_utils.cpp, line 46 > > > > > > We try to avoid using Option> since the semantics is not > > clear between None() and an empty vector. >

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jie Yu
> On Jan. 25, 2016, 9:45 p.m., Jie Yu wrote: > > src/common/command_utils.cpp, line 46 > > > > > > We try to avoid using Option> since the semantics is not > > clear between None() and an empty vector. >

Re: Review Request 42662: Added common command utils file.

2016-01-25 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42662/ --- (Updated Jan. 26, 2016, 2:18 a.m.) Review request for mesos and Jie Yu.

Review Request 42662: Added common command utils file.

2016-01-22 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42662/ --- Review request for mesos and Jie Yu. Repository: mesos Description ---