mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rojas" <alexan...@mesosphere.io>
Subject Re: Review Request 33154: Added reason metrics for slave removals.
Date Fri, 17 Apr 2015 12:38:49 GMT

-----------------------------------------------------------
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
> 
>


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