> On April 10, 2018, 6:15 p.m., Greg Mann wrote: > > src/slave/containerizer/mesos/containerizer.cpp > > Line 2335 (original), 2336 (patched) > > <https://reviews.apache.org/r/66510/diff/1/?file=1994282#file1994282line2336> > > > > Hmm I think it might be a bit more readable here and elsewhere to > > continue using a trivial lambda which returns a default-constructed > > `ContainerTermination`, WDYT? > > > > ``` > > .then([]() { return ContainerTermination(); }); > > ``` > > Andrei Budnik wrote: > `Option<ContainerTermination>::some` is a wrapper for > `ContainerTermination` type: > https://github.com/apache/mesos/blob/3dcd225dbb817da676ae57f4f8871650ece5a0b8/3rdparty/stout/include/stout/option.hpp#L36-L39 > So, this code returns a wrapped termination variable instead of > constructing a new one. > > BTW, the shortest equivalent code would be: > ``` > .then([](const Future<ContainerTermination>& termination) -> > Option<ContainerTermination> { > return termination.get(); > }); > ``` > which doesn't look simpler to me.
Ah OK sorry, makes sense. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66510/#review200828 ----------------------------------------------------------- On April 9, 2018, 3:44 p.m., Andrei Budnik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66510/ > ----------------------------------------------------------- > > (Updated April 9, 2018, 3:44 p.m.) > > > Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian > Zhang. > > > Bugs: MESOS-8706 > https://issues.apache.org/jira/browse/MESOS-8706 > > > Repository: mesos > > > Description > ------- > > This patch changes the return type of `destroy()` method for all > containerizers. As a result, `destroy()` and `wait()` methods have > got the same signature and return type. In fact, all these methods > depend on the same container termination promise, so this unification > makes both containerizer interface and internal logic more consistent. > > > Diffs > ----- > > src/slave/containerizer/composing.hpp > 1221d9b61b7cfbdbaa560dbafb0158225331ed6d > src/slave/containerizer/composing.cpp > cd840a5409023d32701329bf37806f2e7ed6ffd2 > src/slave/containerizer/containerizer.hpp > 836283aa8aa827459558d567e090a230c32351ed > src/slave/containerizer/docker.hpp f1b56c8edced17e9786c86adb9b92c829ed81635 > src/slave/containerizer/docker.cpp 31d64a7ea2a331a084c0f1707b82e938dfe6390f > src/slave/containerizer/mesos/containerizer.hpp > cba4ed2e20622070b46071f35907d8b32c25b2fc > src/slave/containerizer/mesos/containerizer.cpp > 7473a871e626833733f39375b778aff70529dc63 > src/slave/http.cpp d500fde22d01d2c66104b72ff39d9de4e3a411cd > > > Diff: https://reviews.apache.org/r/66510/diff/2/ > > > Testing > ------- > > Tests can't be compiled after this change. > See the next patch in the chain. > > > Thanks, > > Andrei Budnik > >
