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

Ship it!


Some stuff leftover from the last review comments, I'll make the updates and 
commit for you shortly.


src/linux/perf.cpp (line 393)
<https://reviews.apache.org/r/37416/#comment152961>

    two space indent for .then



src/linux/perf.cpp (line 474)
<https://reviews.apache.org/r/37416/#comment152962>

    if (



src/linux/perf.cpp (lines 474 - 482)
<https://reviews.apache.org/r/37416/#comment152965>

    Hm.. you didn't add the explicit discard here?
    
    ```
      if (!version.isReady()) {
        if (version.isFailed()) {
          LOG(ERROR) << "Failed to get perf version: " << version.failure();
        } else {
          LOG(ERROR) << "Failed to get perf version: timeout of 5secs exceeded";
        }
    
        version.discard();
        return false;
      }
    ```



src/linux/perf.cpp (line 478)
<https://reviews.apache.org/r/37416/#comment152964>

    This case also needs to return!



src/linux/perf.cpp (line 479)
<https://reviews.apache.org/r/37416/#comment152963>

    extra space here?


- Ben Mahler


On Aug. 31, 2015, 10:27 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37416/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 10:27 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Perf supported() should be based on the version of perf, not the version of 
> the kernel.
> 
> 
> Diffs
> -----
> 
>   src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 
> 
> Diff: https://reviews.apache.org/r/37416/diff/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Reply via email to