> On Sept. 3, 2020, 1:31 a.m., Benjamin Mahler wrote: > > include/mesos/type_utils.hpp > > Lines 119-122 (original) > > <https://reviews.apache.org/r/72833/diff/1/?file=2239007#file2239007line119> > > > > 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.
Thanks! > On Sept. 3, 2020, 1:31 a.m., Benjamin Mahler wrote: > > src/tests/api_tests.cpp > > Lines 3354-3358 (patched) > > <https://reviews.apache.org/r/72833/diff/1/?file=2239009#file2239009line3354> > > > > Should we just have DEFAULT_FRAMEWORK_INFO explicitly set it to false? > > Just a thought.. Yes, I thought about it; decided to leave it like this. Tests that verify that a non-set `checkpoint` is handled correctly might implicitly depend on this; if we set it to false in `DEFAULT_FRAMEWORK_INFO`, they might just stop doing anything meaningful. - Andrei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72833/#review221787 ----------------------------------------------------------- On Sept. 3, 2020, 5:34 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72833/ > ----------------------------------------------------------- > > (Updated Sept. 3, 2020, 5:34 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/2/ > > > Testing > ------- > > `support/mesos-gtest-runner.py src/mesos-tests` > > > Thanks, > > Andrei Sekretenko > >
