> On July 6, 2018, 12:47 a.m., Chun-Hung Hsiao wrote: > > src/resource_provider/registry.hpp > > Lines 38-50 (patched) > > <https://reviews.apache.org/r/67671/diff/1/?file=2042695#file2042695line38> > > > > We could write the code in a style similar to > > `src/common/type_utils.cpp` is: > > ``` > > return left.id() == right.id() && > > (!left.has_type() || !right.has_type() || left.type() == > > right.type()) && > > (!left.has_name() || !right.has_name() || left.name() == > > right.name())); > > ``` > > > > But whether this is more readable or not is subjective so not opening > > an issue here.
I am leaving this as is as I find above form pretty hard to parse due to the deep nesting and the not apparent coupling between parameters (we also shouldn't assume that applying De Morgan's law is second nature for every reader or maintainer of this code). > On July 6, 2018, 12:47 a.m., Chun-Hung Hsiao wrote: > > src/resource_provider/registry.hpp > > Lines 66 (patched) > > <https://reviews.apache.org/r/67671/diff/1/?file=2042695#file2042695line66> > > > > Would `google::protobuf::util::MessageToJsonString` be better here? Possibly, but this is only used in a single place in the code while we use the debug repr everywhere else. Suggest to leave as is, and possibly perform a global cleanup if we really care. Dropping. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67671/#review205772 ----------------------------------------------------------- On July 6, 2018, 10:50 a.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67671/ > ----------------------------------------------------------- > > (Updated July 6, 2018, 10:50 a.m.) > > > Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. > > > Bugs: MESOS-8838 > https://issues.apache.org/jira/browse/MESOS-8838 > > > Repository: mesos > > > Description > ------- > > Since the agent uses e.g., a resource provider's name or type to > construct paths to persist resource provider state, changes to this > information on resource provider resubscription are not supported. > > This patch persists a resource provider's name and type in the > resource provider registry and rejects a resource provider > resubscription if incompatible changes are detected. Since we did not > persist this information previous to mesos-1.7.0 we cannot and do not > perform validation against resource provider registry information > stored with earlier versions of Mesos. > > > Diffs > ----- > > src/resource_provider/manager.cpp 6400e707ee834c73d4bf18c1f8d8344e8cf714cc > src/resource_provider/registrar.hpp > ded56e1c24a1f8b8db8dc70151682a7deb9e6544 > src/resource_provider/registrar.cpp > a855a2b61fdbfc96a12a133c702ecaf7af666d8b > src/resource_provider/registry.hpp 4c6c4d40686e3af8bbc7affbe3fa673479cc2fbf > src/resource_provider/registry.proto > 491263ef679cd4cea59338b002c6845d890f8eae > src/tests/resource_provider_manager_tests.cpp > 58bdbf0e88f59b5cb0cad42e38b24029fc0f2b41 > > > Diff: https://reviews.apache.org/r/67671/diff/2/ > > > Testing > ------- > > `sudo make check` > > > Thanks, > > Benjamin Bannier > >
