----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44424/#review127329 -----------------------------------------------------------
Thanks for bearing with me. Most of the new comments are from the previous version itself. My bad, should have been caught by me earlier. src/launcher/http_command_executor.cpp (line 240) <https://reviews.apache.org/r/44424/#comment190612> We have typically have the `set_type()` invocation immediatly after defining the `Call` itself. This helps readability and is also consistent with the rest of the code. What I am suggesting is something like this: ```cpp Call call; call.set_type(Call::SUBSCRIBE); call.mutable_framework_id.... ``` Here and all the other places please. src/launcher/http_command_executor.cpp (line 272) <https://reviews.apache.org/r/44424/#comment190615> Would be good to have a comment here too similar to others for consistency. ```cpp // Capture the task. ``` src/launcher/http_command_executor.cpp (line 287) <https://reviews.apache.org/r/44424/#comment190619> This should fit in one line, no? src/launcher/http_command_executor.cpp (line 296) <https://reviews.apache.org/r/44424/#comment190620> Should fit in one line, no? src/launcher/http_command_executor.cpp (line 578) <https://reviews.apache.org/r/44424/#comment190606> s/_task/task.get() src/launcher/http_command_executor.cpp (line 604) <https://reviews.apache.org/r/44424/#comment190608> s/killTask/kill src/launcher/http_command_executor.cpp (line 628) <https://reviews.apache.org/r/44424/#comment190609> s/killTask/kill src/launcher/http_command_executor.cpp (line 709) <https://reviews.apache.org/r/44424/#comment190610> s/killTask/kill src/launcher/http_command_executor.cpp (line 923) <https://reviews.apache.org/r/44424/#comment190602> Nit: s/mesos::v1::FrameworkID/FrameworkID You already have a `using` declaration for this. src/launcher/http_command_executor.cpp (line 924) <https://reviews.apache.org/r/44424/#comment190603> Ditto - Anand Mazumdar On April 5, 2016, 12:36 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44424/ > ----------------------------------------------------------- > > (Updated April 5, 2016, 12:36 p.m.) > > > Review request for mesos, Anand Mazumdar and Vinod Kone. > > > Bugs: MESOS-3558 > https://issues.apache.org/jira/browse/MESOS-3558 > > > Repository: mesos > > > Description > ------- > > Updated http_command_executor.cpp to use v1 API. > > > Diffs > ----- > > src/launcher/http_command_executor.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/44424/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Qian Zhang > >
