----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72833/#review221787 -----------------------------------------------------------
Fix it, then Ship it! include/mesos/type_utils.hpp Lines 119-122 (original) <https://reviews.apache.org/r/72833/#comment310860> Hm.. Can we actually leave these but as `delete`d functions? Seems like C++ does allow non-member functions to be deleted? https://abseil.io/tips/143 https://stackoverflow.com/questions/42332777/what-is-the-point-of-using-delete-on-a-non-member-function ``` // This has been deleted in favor of ... inline bool operator==(const FrameworkInfo& left, const FrameworkInfo& right) = delete; ``` That will make it a lot less likely that someone re-adds this operator when they find it's missing. Also helps them find the equivalent function. src/tests/api_tests.cpp Lines 3354-3358 (patched) <https://reviews.apache.org/r/72833/#comment310861> Should we just have DEFAULT_FRAMEWORK_INFO explicitly set it to false? Just a thought.. src/tests/master_allocator_tests.cpp Lines 374 (patched) <https://reviews.apache.org/r/72833/#comment310862> Not sure that this test needs to be concerned about checking the framework info equality on addFramework calls, it could just check the framework id matches, but even that doesn't seem necessary for this test? Especially since not checking or checking just the ID wouldn't require a custom matcher. - Benjamin Mahler On Sept. 1, 2020, 7:45 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72833/ > ----------------------------------------------------------- > > (Updated Sept. 1, 2020, 7:45 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-10166 > https://issues.apache.org/jira/browse/MESOS-10166 > > > Repository: mesos > > > Description > ------- > > This patch removes the outdated `operator==` for `FrameworkInfo` > (which has been comparing only `name` and `user` fields) > and replaces it with `equivalent()` in the tests that need to compare > `FrameworkInfo`s. > > > Diffs > ----- > > include/mesos/type_utils.hpp da9fd9694b08827b968514b4b078097c912640c8 > include/mesos/v1/mesos.hpp d8304f10d6ac6bbcf014e515e0bfd77a2b96e138 > src/tests/api_tests.cpp e0ce0335bf99f1f624efcca8c86f797df472d68f > src/tests/master_allocator_tests.cpp > 416b7ba733c3a9fc75e64fecf088ff13548bab3f > src/tests/resources_tests.cpp bd044310e297174155b88d5694641acd5df4cf59 > > > Diff: https://reviews.apache.org/r/72833/diff/1/ > > > Testing > ------- > > `support/mesos-gtest-runner.py src/mesos-tests` > > > Thanks, > > Andrei Sekretenko > >
