You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2017/04/03 07:57:59 UTC
Review Request 58136: Cleaned up pattern matching in Resource
operator==.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58136/
-----------------------------------------------------------
Review request for mesos and Jie Yu.
Repository: mesos
Description
-------
The usual pattern for checking optional fields here is to check
whether the field is is either set for both sides or unset for both.
Subsequentially we check if possible values are equal, returning
'false' if they are not.
The code changed here did not follow this pattern, but instead assumed
it was the last check to run. This made it possible that a careless
addition after these checks would never be called.
Diffs
-----
src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a
src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11
Diff: https://reviews.apache.org/r/58136/diff/1/
Testing
-------
Thanks,
Benjamin Bannier
Re: Review Request 58136: Cleaned up pattern matching in Resource
operator==.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58136/#review170843
-----------------------------------------------------------
Patch looks great!
Reviews applied: [58136]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On April 3, 2017, 7:57 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58136/
> -----------------------------------------------------------
>
> (Updated April 3, 2017, 7:57 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The usual pattern for checking optional fields here is to check
> whether the field is is either set for both sides or unset for both.
> Subsequentially we check if possible values are equal, returning
> 'false' if they are not.
>
> The code changed here did not follow this pattern, but instead assumed
> it was the last check to run. This made it possible that a careless
> addition after these checks would never be called.
>
>
> Diffs
> -----
>
> src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a
> src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11
>
>
> Diff: https://reviews.apache.org/r/58136/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 58136: Cleaned up pattern matching in Resource
operator==.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58136/#review174276
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On April 3, 2017, 7:57 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58136/
> -----------------------------------------------------------
>
> (Updated April 3, 2017, 7:57 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The usual pattern for checking optional fields here is to check
> whether the field is is either set for both sides or unset for both.
> Subsequentially we check if possible values are equal, returning
> 'false' if they are not.
>
> The code changed here did not follow this pattern, but instead assumed
> it was the last check to run. This made it possible that a careless
> addition after these checks would never be called.
>
>
> Diffs
> -----
>
> src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a
> src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11
>
>
> Diff: https://reviews.apache.org/r/58136/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benjamin Bannier
>
>