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



src/master/master.hpp
<https://reviews.apache.org/r/33154/#comment130360>

    I find this quite counter intuitive for two reasons:
    1. If I just check this header, I would expect reason to behave a little 
like an enum, telling me why the slave is being removed. What i didn't expect, 
is that it is a counter to the number of slaves being removed by `reason`.
    2. The parameter is a const ref, last thing I expected is that it would 
change later on.
    
    Can you add a comment explaining the semantics of reason, and if possible, 
getting rid of the const ref?


- Alexander Rojas


On April 14, 2015, 3:46 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33154/
> -----------------------------------------------------------
> 
> (Updated April 14, 2015, 3:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2485
>     https://issues.apache.org/jira/browse/MESOS-2485
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See [MESOS-2485](https://issues.apache.org/jira/browse/MESOS-2485).
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
>   src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36 
>   src/master/metrics.hpp 52a83289cfe7e6b6fd8d5bff0774ebe5ce51d0ed 
>   src/master/metrics.cpp 14486bf7130250f561c9fb7a43c95f3fc1e76a4b 
> 
> Diff: https://reviews.apache.org/r/33154/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Reply via email to