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


Mostly comments from the last review


src/linux/perf.cpp (lines 295 - 296)
<https://reviews.apache.org/r/37045/#comment148811>

    This still isn't lined up?



src/linux/perf.cpp (line 297)
<https://reviews.apache.org/r/37045/#comment148813>

    I think we're trying to avoid the blanket '=' if possible, looks like you 
only use 'this' here? Does this work if you capture 'this' only?



src/linux/perf.cpp (line 301)
<https://reviews.apache.org/r/37045/#comment148817>

    Please add an explicit CHECK that this is not discarded, rather than 
relying on .get crashing



src/linux/perf.cpp (line 303)
<https://reviews.apache.org/r/37045/#comment148816>

    Let's print the .failure of the future if it's failed.



src/linux/perf.cpp (lines 308 - 309)
<https://reviews.apache.org/r/37045/#comment148819>

    No period for composition per the last comment, also WSTRINGIFY will print 
'exited with status' already if appropriate, so this is double logging it.



src/linux/perf.cpp (line 314)
<https://reviews.apache.org/r/37045/#comment148818>

    Please add an explicit CHECK that this is not discarded (otherwise calling 
.failure / .get crashing)


- Ben Mahler


On Aug. 5, 2015, 5:25 a.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37045/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2015, 5:25 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
>     https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Convert Linux perf sampler to use process:await().
> 
> 
> Diffs
> -----
> 
>   src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 
> 
> Diff: https://reviews.apache.org/r/37045/diff/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Reply via email to