> On June 24, 2015, 3:13 a.m., Bernd Mathiske wrote:
> > src/tests/docker_tests.cpp, line 374
> > <https://reviews.apache.org/r/35799/diff/1/?file=990751#file990751line374>
> >
> > Use Flags::docker_sandbox_directory (or a constant that the flag also
> > uses for its default value) instead. Not only will this fix the naked
> > constant problem, it will also explain what this parameter does and make
> > readers understand the test code much more quickly.
> >
> > I know this is copy-paste-induced, but the original is really, really
> > bad. Please either fix now, or leave a TODO and a tech debt ticket.
>
> Timothy Chen wrote:
> We actually put anything here, so I just put the default settings. I can
> just use /tmp/sandbox if that makes it more obvious?
What you have put there now ("directory.get()") does not provide the user with
an idea what is going on without looking up the definition of the parameters of
Docker::run() (nor would "/tmp/sandbox"). This may seem like a moot point,
since it is indeed arbitrary what to put there. But conveying that information
to the reader is actually the real point in question. So this fix is worse than
the original IMHO.
The previous string constant actually gave more of a hint and now I understand
now why you named it "/mnt/mesos/sandbox". This kinda stands for
"/arbitrary/mount/point/in/docker/for/the/Mesos/sandbox". Sorry for opening the
issue in the first place! Please revert to "/mnt/mesos/sandbox". (Maybe add a
helpful source code comment? :-)
- Bernd
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35799/#review89147
-----------------------------------------------------------
On June 26, 2015, 4:26 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35799/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 4:26 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till
> Toenshoff.
>
>
> Bugs: MESOS-2374
> https://issues.apache.org/jira/browse/MESOS-2374
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Support mounting relative paths with docker.
>
>
> Diffs
> -----
>
> src/docker/docker.cpp 62aac2773d8b74e19619cf593064165892b03ba6
> src/tests/docker_tests.cpp acf1e3c8f455e8b40e4b3cb8748fcfeacbab142c
>
> Diff: https://reviews.apache.org/r/35799/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
