> On Feb. 1, 2017, 11:38 p.m., Michael Park wrote: > > src/master/master.hpp, lines 2441-2450 > > <https://reviews.apache.org/r/56004/diff/1/?file=1617175#file1617175line2441> > > > > Could we just follow this pattern? > > > > ```cpp > > if (source.has_role()) { > > info.set_role(source.role()); > > } else { > > info.clear_role(); > > } > > > > if (source.roles_size() > 0) { > > info.mutable_roles()->CopyFrom(source.roles()); > > } else { > > info.clear_roles(); > > } > > ```
I found this harder to follow. The one I went with was: wipe the fields and then set based on source. The suggestion here seems to be: if the source has it set, overwrite, otherwise wipe, which just seemed a little less clear to me. Thoughts? - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56004/#review163900 ----------------------------------------------------------- On Jan. 27, 2017, 12:30 a.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56004/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2017, 12:30 a.m.) > > > Review request for mesos, Benjamin Bannier and Michael Park. > > > Repository: mesos > > > Description > ------- > > The first issue is that we need to update the capabilities member > to reflect the new capabilities. > > The second issue is that when we allow an upgrade or downgrade > to or from MULTI_ROLE, we need to update the `role` and `roles` > fields of `FrameworkInfo`. > > > Diffs > ----- > > src/master/master.hpp 7e38af41ca16241dbbe3bc2e80c0848e82762a45 > > Diff: https://reviews.apache.org/r/56004/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Mahler > >
