Re: Review Request 43453: Added shell flag to mesos-execute to run image default config.

2016-02-10 Thread Jie Yu 

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


Ship it!




Ship It!

- Jie Yu


On Feb. 11, 2016, 12:43 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43453/
> ---
> 
> (Updated Feb. 11, 2016, 12:43 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added shell flag to mesos-execute to run image default config.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 4b2244c7449cbba53e4c280c4474557d240b8038 
> 
> Diff: https://reviews.apache.org/r/43453/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43453: Added shell flag to mesos-execute to run image default config.

2016-02-10 Thread Gilbert Song 

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

(Updated Feb. 10, 2016, 4:43 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added shell flag to mesos-execute to run image default config.


Diffs (updated)
-

  src/cli/execute.cpp 4b2244c7449cbba53e4c280c4474557d240b8038 

Diff: https://reviews.apache.org/r/43453/diff/


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 43453: Added shell flag to mesos-execute to run image default config.

2016-02-10 Thread Jie Yu 

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




src/cli/execute.cpp (lines 70 - 71)


Just say if the command is a shell or not. If not, 'command' will be 
treated as value and arguments (TODO).



src/cli/execute.cpp (line 208)


use CHECK_SOME(command) here



src/cli/execute.cpp (lines 209 - 211)


Comments here does not sound necessary. Please kill it.



src/cli/execute.cpp (line 213)


kill this line



src/cli/execute.cpp (lines 216 - 217)


Why only possible if docker_image is specified? Please remove this comments



src/cli/execute.cpp (line 218)


Please add a TODO here to treat 'command' as value and arguments.



src/cli/execute.cpp (line 326)


Can you move 'shell' right above 'command'.

Please be consistent and fix other orders as well.


- Jie Yu


On Feb. 10, 2016, 11:44 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43453/
> ---
> 
> (Updated Feb. 10, 2016, 11:44 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added shell flag to mesos-execute to run image default config.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 4b2244c7449cbba53e4c280c4474557d240b8038 
> 
> Diff: https://reviews.apache.org/r/43453/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 43453: Added shell flag to mesos-execute to run image default config.

2016-02-10 Thread Gilbert Song 

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added shell flag to mesos-execute to run image default config.


Diffs
-

  src/cli/execute.cpp 4b2244c7449cbba53e4c280c4474557d240b8038 

Diff: https://reviews.apache.org/r/43453/diff/


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song