mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 35473: Removed a few incorrect CHECKs in DRF sorter.
Date Mon, 15 Jun 2015 22:17:07 GMT

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

Ship it!


I personally find it easier to read if we don't have the special 'return' cases:

```
{
  if (!resources.empty()) {
    // Do work.
  }
}

Seems clearer than:
{
  if (resources.empty()) {
    // Don't do work by returning.
  }
  
  // Do work.
}
```

I think most of the cases where we've done the early returns were motivated by a few reasons
that don't hold here:

(1) We wanted to avoid deep nesting of logic (using 'returns' allowed us to flatten the logical
structure), and/or
(2) We wanted to eliminate many special cases in sequence, until only the correct case remains
(e.g. message handlers, (a) can't lookup framework, (b) message sent by wrong pid, (c) invalid
offers used, ...).

Up to you!


src/tests/examples_tests.cpp
<https://reviews.apache.org/r/35473/#comment140390>

    Don't forget to delete this


- Ben Mahler


On June 15, 2015, 10:03 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35473/
> -----------------------------------------------------------
> 
> (Updated June 15, 2015, 10:03 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2627
>     https://issues.apache.org/jira/browse/MESOS-2627
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removed a few incorrect CHECKs in DRF sorter.
> 
> See details in the ticket for the reason why this is needed.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/sorter/drf/sorter.cpp ac05afdc7d408735dd796faa68c943e75540aaa7

>   src/tests/examples_tests.cpp 41a9265e91ea54b657afbd32c22e204c5f64ca17 
> 
> Diff: https://reviews.apache.org/r/35473/diff/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


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