----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26517/#review58938 -----------------------------------------------------------
src/slave/containerizer/docker.hpp <https://reviews.apache.org/r/26517/#comment100125> why not do this as a const char * / compile time string literal rather than runtime? I see this as a fairly common pattern in our codebase, just doesn't make a lot of sense to me. In any case, the variable is expected to be set only at construction and never changed, so it would be good to make it const. 'extern const std::string'. We may want to look at changing these variables to static functions so that there is a well defined initialization order. src/slave/containerizer/docker.cpp <https://reviews.apache.org/r/26517/#comment100129> I get why this cleanup should be in the destructor, but it feels uneven that we do the cleanup in the destructor but we don't do the creation / symlink in the constructor. Destructors should roughly reverse constructors. src/slave/containerizer/docker.cpp <https://reviews.apache.org/r/26517/#comment100161> Indentation should line up like: Error(" " - Cody Maloney On Oct. 29, 2014, 12:25 a.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26517/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2014, 12:25 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-1833 > https://issues.apache.org/jira/browse/MESOS-1833 > > > Repository: mesos-git > > > Description > ------- > > Review: https://reviews.apache.org/r/26517 > > > Diffs > ----- > > src/slave/containerizer/docker.hpp fbbd45d > src/slave/containerizer/docker.cpp 9a29489 > src/tests/docker_containerizer_tests.cpp 67d60a8 > > Diff: https://reviews.apache.org/r/26517/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Timothy Chen > >
