----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57059/#review166800 -----------------------------------------------------------
LGTM, holding off the ship it due to a minor comment/query around reordering expectations. src/tests/default_executor_tests.cpp (line 142) <https://reviews.apache.org/r/57059/#comment238875> Not yours: Can we directly do this later instead of creating `frameworkInfo` at all here? ```cpp subscribe->mutable_framework_info()->CopyFrom(v1::DEFAULT_FRAMEWORK_INFO); ``` src/tests/default_executor_tests.cpp (lines 155 - 166) <https://reviews.apache.org/r/57059/#comment238877> Might want to group all the `executorInfo` statements together for readability? ```cpp v1::FrameworkID frameworkId(subscribed->framework_id()); v1::Resources resources = v1::Resources::parse("cpus:0.1;mem:32;disk:32").get(); v1::ExecutorInfo executorInfo; executorInfo.set_type(v1::ExecutorInfo::DEFAULT); executorInfo.mutable_executor_id()->CopyFrom(v1::DEFAULT_EXECUTOR_ID); executorInfo.mutable_framework_id()->CopyFrom(frameworkId); executorInfo.mutable_resources()->CopyFrom(resources); ``` src/tests/default_executor_tests.cpp (lines 308 - 312) <https://reviews.apache.org/r/57059/#comment238879> hmm, do we want to move all the expectations just before sending the call itself i.e. before L337? Ditto for the other occurences too. - Anand Mazumdar On Feb. 25, 2017, 2:02 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57059/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2017, 2:02 a.m.) > > > Review request for mesos, Anand Mazumdar and Gilbert Song. > > > Repository: mesos > > > Description > ------- > > Reorganized so that objects are defined closer to their usage. > > > Diffs > ----- > > src/tests/default_executor_tests.cpp > eaf639467aaaa35b28b21bfb7e16aca5924a5a82 > > Diff: https://reviews.apache.org/r/57059/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
