> On Jan. 8, 2015, 11:27 a.m., Bernd Mathiske wrote: > > src/docker/executor.cpp, line 165 > > <https://reviews.apache.org/r/29329/diff/1/?file=798951#file798951line165> > > > > 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.)
It is a general assumption across everywhere using subprocess, I can put it here again. - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29329/#review67035 ----------------------------------------------------------- On Jan. 6, 2015, 10: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, 10: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 > >
