> On May 17, 2019, 10:16 a.m., Greg Mann wrote: > > src/tests/api_tests.cpp > > Lines 2774 (patched) > > <https://reviews.apache.org/r/70651/diff/1/?file=2145475#file2145475line2777> > > > > Nit: make this a const ref? > > Chun-Hung Hsiao wrote: > This is copied because `event` is overwritten later. Do you think that > it's okay to use a ref, since we don't access to it afterwards? > > Greg Mann wrote: > As long as `agentAdded` isn't modified, we can make it a const ref? I was > suggesting we do the same thing with `agentAdded` that we do previously in > this test with `getState`: > ``` > const v1::master::Response::GetState& getState = > event->get().subscribed().get_state(); > ``` > > Chun-Hung Hsiao wrote: > The problem is that `event->get()` will be destructed once `event` is > overwritten, so the ref, const or not, will no longer be valid. So making a > copy is a bit safer (if anyone touches this test in the future). > > Greg Mann wrote: > But it looks like we already use the const ref pattern in this test? It > seems best to me to just be consistent, but I don't have strong feelings. If > you don't want to change it, you can drop the issues.
Given that it's not very likely we'll change this test to access these references in the future, I'll take your suggestion for consistency :) - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70651/#review215319 ----------------------------------------------------------- On May 16, 2019, 5:11 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70651/ > ----------------------------------------------------------- > > (Updated May 16, 2019, 5:11 a.m.) > > > Review request for mesos and Greg Mann. > > > Bugs: MESOS-9785 > https://issues.apache.org/jira/browse/MESOS-9785 > > > Repository: mesos > > > Description > ------- > > If one subscribes to master's `/api/v1` endpoint after a master failover > but before an agent reregistration, frameworks recovered through the > agent registration should be notified to the subscriber, otherwise > recovered tasks will have framework IDs referring to frameworks unknown > to the subscriber. > > > Diffs > ----- > > src/common/protobuf_utils.cpp 8b252cb11e17356836988dfc44a63953579a1def > src/master/master.cpp 6c0e30bff3fbfe0ad1f3af44eb3df6f82048f0e1 > src/tests/api_tests.cpp bc19d7e9661f091aebf3072967548c7c3196239d > > > Diff: https://reviews.apache.org/r/70651/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Chun-Hung Hsiao > >
