----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60928/#review180939 -----------------------------------------------------------
Looks good! Mostly cleanup comments. include/mesos/master/master.proto Lines 515 (patched) <https://reviews.apache.org/r/60928/#comment256291> Nit, How about: ```cpp // This can happen when a new framework registers with the master. ``` src/common/protobuf_utils.cpp Lines 817 (patched) <https://reviews.apache.org/r/60928/#comment256296> Can you save `event.mutable_framework_added()->mutable_framework()` in a pointer variable first? Have a look at `createTaskUpdated()` on how to go about it. This would help readability. src/common/protobuf_utils.cpp Lines 819-825 (patched) <https://reviews.apache.org/r/60928/#comment256299> hmm, Is this necessary? When a new framework is added, `active`/`connected` would be true and `recovered` would be `false`? src/master/master.cpp Lines 2704-2708 (patched) <https://reviews.apache.org/r/60928/#comment256300> hmm, why not do this inside `addFramework()` to be consistent with how `TASK_ADDED` and `TASK_UPDATED` events are being sent i.e., from `addTask()` and `updateTask()`? src/master/master.cpp Lines 3005 (patched) <https://reviews.apache.org/r/60928/#comment256301> Kill this; The line is self explanatory. - Anand Mazumdar On July 18, 2017, 12:11 a.m., Quinn Leng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60928/ > ----------------------------------------------------------- > > (Updated July 18, 2017, 12:11 a.m.) > > > Review request for mesos, Anand Mazumdar, Greg Mann, Vinod Kone, and Zhitao > Li. > > > Bugs: MESOS-6101 > https://issues.apache.org/jira/browse/MESOS-6101 > > > Repository: mesos > > > Description > ------- > > Added event 'FRAMEWORK_ADDED' for master's v1 streaming operator API. > It will be generated when the framework registers for the first time. > > > Diffs > ----- > > include/mesos/master/master.proto 7a722a6deac484f28a85f63fd68ef49cb6a47606 > include/mesos/v1/master/master.proto > cf88ea64fa08e4bdcf685de4def28d374eb32fdf > src/common/protobuf_utils.hpp 2156f6d158bfee16d29aa3531bdd156af581df01 > src/common/protobuf_utils.cpp 4e5ab02c90ad8490efbcc471f7fd7c48be0c4678 > src/master/master.cpp d895154c00eef17511c46ddd0b75922f952707eb > > > Diff: https://reviews.apache.org/r/60928/diff/4/ > > > Testing > ------- > > make check > > > Thanks, > > Quinn Leng > >
