> On April 19, 2015, 11:27 p.m., Adam B wrote:
> > src/cli/execute.cpp, line 77
> > <https://reviews.apache.org/r/33109/diff/2/?file=924729#file924729line77>
> >
> >     Any reason not to just name this 'env' or 'environment'?
> 
> haosdent huang wrote:
>     I afraid "environment/env" maybe have other usage in the future. But if 
> you think "environment" is ok, I would change it here.

Should be fine. 'mesos-execute' is very specific to launching a single command.


> On April 19, 2015, 11:27 p.m., Adam B wrote:
> > src/cli/execute.cpp, line 396
> > <https://reviews.apache.org/r/33109/diff/2/?file=924729#file924729line396>
> >
> >     How is this going to work with environment variables like PATH that 
> > expect :'s inside their values?
> >     
> >     Probably need to choose another delimiter (even `;` and `=` could be 
> > tricky), or pass in a newline-delimited file, or go all the way to json 
> > lists.
> 
> haosdent huang wrote:
>     How about pass a json list here?

SGTM. You want want to enable reading the json in from a file too, in case it 
gets long. See how the rest of Mesos reads in json parameters in master/slave 
flags.


- Adam


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


On April 13, 2015, 9:42 a.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33109/
> -----------------------------------------------------------
> 
> (Updated April 13, 2015, 9:42 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-2023
>     https://issues.apache.org/jira/browse/MESOS-2023
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Allow setting environment variables in mesos-execute
> 
> 
> Diffs
> -----
> 
>   src/cli/execute.cpp 84f70dccbc2c5dd43f68105d967f4488c82f582b 
> 
> Diff: https://reviews.apache.org/r/33109/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Reply via email to