> On Feb. 2, 2018, 9:38 p.m., Andrew Schwartzmeyer wrote:
> > src/checks/checker_process.cpp
> > Lines 523-525 (patched)
> > <https://reviews.apache.org/r/65396/diff/2/?file=1950446#file1950446line523>
> >
> >     It's kind of funny they're quoting this. Not something to fix here, but 
> > it's weird to have hard-coded `sh -c`.
> >     
> >     Joe and I have talked about making the shell switchable (so you can 
> > switch between cmd.exe and PowerShell on Windows, or sh and bash etc. on 
> > Linux). Maybe we should file an issue so can start tracking hard-coded 
> > shells with a `TODO`?

Yeah that sounds good to me. It's going to call os::Shell in the next patch, so 
the hard-coded `sh -c` is going to go away.


> On Feb. 2, 2018, 9:38 p.m., Andrew Schwartzmeyer wrote:
> > src/checks/checker_process.cpp
> > Lines 529 (patched)
> > <https://reviews.apache.org/r/65396/diff/2/?file=1950446#file1950446line529>
> >
> >     Is `command.arguments()` iterable? If so, we could use the vector's 
> > reange constructor.

we can do `commandArguments.insert(commandArguments.end(), 
command.arguments().begin(), command.arguments().end());`. But since it's going 
to changed anyway in the next patch, I'll do it there.


- Akash


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


On Feb. 8, 2018, 5:50 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65396/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2018, 5:50 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
>     https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Now with the health check library refactor, the wrapping of `docker
> exec` for docker command health checks can be easily moved inside
> the library.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
>   src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
> 
> 
> Diff: https://reviews.apache.org/r/65396/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Reply via email to