----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47363/#review133289 -----------------------------------------------------------
include/mesos/v1/executor.hpp (line 38) <https://reviews.apache.org/r/47363/#comment197577> We don't name our interfaces as *Interface :) Is there an alternative name we could use? Also, I'm curious to know why you used inheritance instead of composition. For example, the Mesos class itself could take a parameter which tells it use V1 or V0 API. include/mesos/v1/executor.hpp (line 74) <https://reviews.apache.org/r/47363/#comment197578> use the override keyword here? src/executor/v0_v1executor.hpp (lines 45 - 71) <https://reviews.apache.org/r/47363/#comment197579> do you want to use the override keywords here as well? src/executor/v0_v1executor.hpp (line 75) <https://reviews.apache.org/r/47363/#comment197580> Any reason why the driver is not a part of the V0ToV1AdapterProcess ? src/executor/v0_v1executor.cpp (lines 66 - 67) <https://reviews.apache.org/r/47363/#comment197573> why did you need to make a copy? src/executor/v0_v1executor.cpp (lines 106 - 107) <https://reviews.apache.org/r/47363/#comment197582> hmm. this seems like a bug. we should fix the driver! src/executor/v0_v1executor.cpp (line 173) <https://reviews.apache.org/r/47363/#comment197574> why did you pull it out instead of doing it inside the case statement? src/executor/v0_v1executor.cpp (line 234) <https://reviews.apache.org/r/47363/#comment197581> s/pendingEvents/pending/ - Vinod Kone On May 14, 2016, 8:39 a.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47363/ > ----------------------------------------------------------- > > (Updated May 14, 2016, 8:39 a.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-5302 > https://issues.apache.org/jira/browse/MESOS-5302 > > > Repository: mesos > > > Description > ------- > > This change adds a driver to v1 executor shim/adapter. This can > be used by the command executor/docker executor to toggle > between using the new/old API instead of duplicating the > code like we did with `http_command_executor.cpp`. > > > Diffs > ----- > > include/mesos/v1/executor.hpp adca287be3bb88c8b3298705740cb6bcbb3a09f0 > src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 > src/executor/v0_v1executor.hpp PRE-CREATION > src/executor/v0_v1executor.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/47363/diff/ > > > Testing > ------- > > make check (Later in the chain, we make the command executor use this > interface) > So the existing tests should be able to test this out. > > > Thanks, > > Anand Mazumdar > >
