mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Sekretenko <asekrete...@apache.org>
Subject Re: Review Request 72833: Removed `bool operator==(const FrameworkInfo&, const FrameworkInfo&)`.
Date Thu, 03 Sep 2020 15:55:11 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message