mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Hindman <b...@berkeley.edu>
Subject Re: Review Request 65150: Added support for discarding a Future from Queue::get.
Date Mon, 15 Jan 2018 20:52:40 GMT

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


Fix it, then Ship it!





3rdparty/libprocess/include/process/queue.hpp
Lines 69 (patched)
<https://reviews.apache.org/r/65150/#comment274613>

    It seems like some mental overhead to realize that `future` should only ever be pending
if it comes from the first branch of the if block above and not the else. Why not just include
this within the if block above?



3rdparty/libprocess/include/process/queue.hpp
Lines 74 (patched)
<https://reviews.apache.org/r/65150/#comment274614>

    I personally prefer nesting these as it's less prone to copy-pasta mistakes, but up to
you.


- Benjamin Hindman


On Jan. 13, 2018, 9:35 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65150/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2018, 9:35 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/queue.hpp 4d1c2cfa811b9377da8631311aaf39634695eae6

> 
> 
> Diff: https://reviews.apache.org/r/65150/diff/1/
> 
> 
> Testing
> -------
> 
> Added a test in the subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


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