> On June 29, 2016, 8:55 p.m., Timothy Chen wrote: > > src/docker/executor.cpp, line 465 > > <https://reviews.apache.org/r/49351/diff/2/?file=1432556#file1432556line465> > > > > wrapHealthCheckCommand sounds very generic and to me it makes it less > > clear what the extent is, so i'm not sure this change makes the code much > > more understandable. > > > > I don't see you need to refactor this in your other patch as well. > > > > My opinion I think is to just keep it as is, or perhaps rename wrap.... > > into something more obvious?
Got it, let me find a better name. I still think it is necessary to break `launchHealthCheck` into smaller functions to make it more readable. - haosdent ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49351/#review140049 ----------------------------------------------------------- On June 29, 2016, 9:03 a.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49351/ > ----------------------------------------------------------- > > (Updated June 29, 2016, 9:03 a.m.) > > > Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and > Timothy Chen. > > > Bugs: MESOS-2533 > https://issues.apache.org/jira/browse/MESOS-2533 > > > Repository: mesos > > > Description > ------- > > Added wrapper function for health check in docker executor. > > > Diffs > ----- > > src/docker/executor.cpp 88b7fc4c36ed3974ac6b103a29e1d975619f0c69 > > Diff: https://reviews.apache.org/r/49351/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
