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


Awesome, are you planning to integrate this into the Registrar too? :D


3rdparty/libprocess/include/process/metrics/timer.hpp
<https://reviews.apache.org/r/20593/#comment74973>

    Can you take a const reference here?
    
    Also, s/f/future/



3rdparty/libprocess/include/process/metrics/timer.hpp
<https://reviews.apache.org/r/20593/#comment74979>

    Alternatively, we could just use the Clock without any additional 
complexity and with better testability (no need to sleep, etc) in tests.
    
    Any reason to prefer Stopwatch? We could move to using Clock::now for 
start()/stop() and it would be easier to test that we're correctly timing 
things.



3rdparty/libprocess/include/process/metrics/timer.hpp
<https://reviews.apache.org/r/20593/#comment74977>

    To be even more explicit, how about:
    
    future
      .onAny(...);
    
    return future;
    
    (Normally we'll place callbacks on separate lines)



3rdparty/libprocess/src/tests/metrics_tests.cpp
<https://reviews.apache.org/r/20593/#comment74980>

    If you just return 42, does the test still pass?


- Ben Mahler


On April 23, 2014, 6:55 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20593/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 6:55 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/metrics/timer.hpp 
> c8e1d91bdc39fb0702bc761e98807abdade4996d 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 6f3795b33b92dd0621867e274649f8b9667ba538 
> 
> Diff: https://reviews.apache.org/r/20593/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Reply via email to