----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18311/#review36738 -----------------------------------------------------------
Ship it! Sorry for the delay on this Till. Just a few minor cleanups then I'll get this committed. Thanks! 3rdparty/libprocess/include/process/collect.hpp <https://reviews.apache.org/r/18311/#comment67926> After the inclusion of Future::after I'm not sure that this will be as useful since if you've already got a future you'll be able to just do 'future.after(Seconds(N), ...)' rather than 'await(future, Seconds(N)).then(...)'. Can you think of another reason why to include this one? If not, how about we kill it? 3rdparty/libprocess/include/process/collect.hpp <https://reviews.apache.org/r/18311/#comment67912> const & for parameter. Also, can we call this __await (as well as for the one below) to capture that these are continuations for await please? 3rdparty/libprocess/include/process/collect.hpp <https://reviews.apache.org/r/18311/#comment67910> Indent another +2 please, and const & for parameter? 3rdparty/libprocess/include/process/collect.hpp <https://reviews.apache.org/r/18311/#comment67914> C++11, kill the spaces between > > please. 3rdparty/libprocess/include/process/collect.hpp <https://reviews.apache.org/r/18311/#comment67916> Kill spaces between '> >' here as well, and else where in C++11 code please. 3rdparty/libprocess/src/tests/process_tests.cpp <https://reviews.apache.org/r/18311/#comment67905> Should be an ASSERT_TRUE since we're trying to test the fact that after 'future' was ready the internal futures have completed and we don't have to wait for them. 3rdparty/libprocess/src/tests/process_tests.cpp <https://reviews.apache.org/r/18311/#comment67906> For the same reason as above, let's use ASSERT_TRUE here. 3rdparty/libprocess/src/tests/process_tests.cpp <https://reviews.apache.org/r/18311/#comment67927> See comments above, let's use ASSERT_TRUE here too please. 3rdparty/libprocess/src/tests/process_tests.cpp <https://reviews.apache.org/r/18311/#comment67928> And finally ASSERT_TRUE here too! - Benjamin Hindman On March 10, 2014, 2:15 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18311/ > ----------------------------------------------------------- > > (Updated March 10, 2014, 2:15 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Repository: mesos-git > > > Description > ------- > > Currently the Process::await implementation on list<Future>, by the nature of > std::list, expects equally typed futures. This new override implements await > for a tuple of futures, hence allows awaiting differently typed futures in a > single call. > > There also is a new override that allows await on a single Future, a > convenience approach for allowing a Process based await on a single Future > without forcing the user to render a list or tuple out of that. > > A C++11 and a boost-based implementation have been added. > > This patch also includes tests on those new overrides. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/collect.hpp 2a73bc9 > 3rdparty/libprocess/src/tests/process_tests.cpp e899aed > > Diff: https://reviews.apache.org/r/18311/diff/ > > > Testing > ------- > > make check (clang c++11, gcc) > > > Thanks, > > Till Toenshoff > >
