----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29329/#review67035 -----------------------------------------------------------
What bug ID does this refer to? Any tests? Does this work already? src/docker/executor.cpp <https://reviews.apache.org/r/29329/#comment110858> The "_" is unnecessary. src/docker/executor.cpp <https://reviews.apache.org/r/29329/#comment111055> This is a major constraint that should be mentioned at the top of the file. src/docker/executor.cpp <https://reviews.apache.org/r/29329/#comment111053> An important constraint/assumption like this should be mentioned at the top of the file. src/docker/executor.cpp <https://reviews.apache.org/r/29329/#comment111049> This cryptic statement is worth a comment (maybe mention "man waitpid"? Or mention higher up in this code that the status code is based on waitpid, which is not immediately evident in this scope.). Also maybe add some more output to increase human parsing ease. For example: CHECK(WIFEXITED(s) || WIFSIGNALED(s)) << "status code: " << s; (I know you just copied this particular stretch of code (from executor.cpp). Is there a general rule that code can or should remain suboptimal if copied? Please feel free to drop this issue if this is the case.) src/docker/executor.cpp <https://reviews.apache.org/r/29329/#comment111056> This stems from copied code. Any ideas how to do this properly? - Bernd Mathiske On Jan. 6, 2015, 2:16 p.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29329/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2015, 2:16 p.m.) > > > Review request for mesos, Benjamin Hindman and Bernd Mathiske. > > > Repository: mesos-git > > > Description > ------- > > Add executor for docker containerizer, replaces the usage of command executor > > > Diffs > ----- > > src/Makefile.am 6f132b5a66e117c2a907763217bfafe1fce1b7a0 > src/docker/executor.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/29329/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Timothy Chen > >
