mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Schlicht <...@mesosphere.io>
Subject Re: Review Request 62842: Fixed 'operator==' for 'v1::Resource::DiskInfo::Source'.
Date Mon, 09 Oct 2017 12:29:50 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62842/#review187401
-----------------------------------------------------------


Ship it!




Ship It!

- Jan Schlicht


On Oct. 9, 2017, 2:13 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62842/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2017, 2:13 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When implementing 'operator==' for protobufs as a pattern we typically
> first check that two 'optional' fields are set for both the left- and
> right-hand side, and only then compare their values, e.g., given a
> definition
> 
>     message Foo {
>       optional string bar = 1;
>     }
> 
> we would implement 'operator==' similar to the following,
> 
>    bool operator==(const Foo& lhs, const Foo& rhs)
>    {
>        if (lhs.has_bar() != rhs.has_bar()) {
>          return false;
>        }
> 
>        if (lhs.has_bar() && lhs.bar() != rhs.bar()) {
>          return false;
>        }
> 
>        return true;
>    }
> 
> One reason for this is that it allows us to distinguish an unset field
> from a set field containing a default constructed value (if e.g.,
> above 'lhs.has_bar()' was 'false', 'lhs.bar()' would return an empty
> string).
> 
> This patch makes sure we use the same pattern when checking
> 'v1::Resource::DiskInfo::Source' for equality. An earlier patch
> ('591f74d') already fixed this for 'Resources::DiskInfo::Source'.
> 
> 
> Diffs
> -----
> 
>   src/v1/resources.cpp a64180bad1d782dd270e06217a389002155c2e10 
> 
> 
> Diff: https://reviews.apache.org/r/62842/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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