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

Ship it!


Looking through git blame and https://issues.apache.org/jira/browse/MESOS-1278, 
this looks safe to remove to me.

The std:: removal could have been an independent patch (would have made it 
easier to review, and it lowers the surface area of the change which helps for 
blaming/reverting). I'll do this for you, but only this time since it's tedious 
on the reviewer and reviewing is our bottleneck. :)

I've also pulled out cleanups around using the -> operator.


src/linux/perf.cpp (lines 66 - 101)
<https://reviews.apache.org/r/37976/#comment153051>

    No need for this to be pulled out now that there's only one code path.



src/linux/perf.cpp (lines 300 - 306)
<https://reviews.apache.org/r/37976/#comment153011>

    Don't need this anymore.



src/linux/perf.cpp (lines 309 - 336)
<https://reviews.apache.org/r/37976/#comment153045>

    No longer a need for the internal sample.



src/tests/containerizer/cgroups_tests.cpp (lines 1001 - 1002)
<https://reviews.apache.org/r/37976/#comment153014>

    You can use an initializer list directly at the call instead of pulling out 
the variable.



src/tests/containerizer/cgroups_tests.cpp (lines 1007 - 1019)
<https://reviews.apache.org/r/37976/#comment153022>

    Let's use the -> operator on Future now that we have it! :)



src/tests/containerizer/cgroups_tests.cpp (line 1008)
<https://reviews.apache.org/r/37976/#comment153021>

    Why did you change this to 4 spaces? Wrapping after = is just 2 spaces.



src/tests/containerizer/cgroups_tests.cpp (line 1011)
<https://reviews.apache.org/r/37976/#comment153015>

    We should also check that it contains the key before calling .at



src/tests/containerizer/perf_tests.cpp (line 27)
<https://reviews.apache.org/r/37976/#comment153050>

    Don't need this anymore :)



src/tests/containerizer/perf_tests.cpp (line 63)
<https://reviews.apache.org/r/37976/#comment153016>

    And here you re-added the std:: prefix? It would be really helpful if you 
could catch these through self-review :)



src/tests/containerizer/perf_tests.cpp (line 72)
<https://reviews.apache.org/r/37976/#comment153048>

    Let's use the -> operator to clean this up, now that we have it



src/tests/containerizer/perf_tests.cpp (lines 91 - 92)
<https://reviews.apache.org/r/37976/#comment153047>

    This fails the test!



src/tests/containerizer/perf_tests.cpp (line 97)
<https://reviews.apache.org/r/37976/#comment153049>

    s/./,/



src/tests/containerizer/perf_tests.cpp (lines 100 - 101)
<https://reviews.apache.org/r/37976/#comment153046>

    This fails the test!


- Ben Mahler


On Aug. 31, 2015, 11:57 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37976/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 11:57 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3347
>     https://issues.apache.org/jira/browse/MESOS-3347
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Simplify perf by removing unused sampling capabilities.
> 
> 
> Diffs
> -----
> 
>   src/linux/perf.hpp b77b61d7048b12cea4586bcf802cbc2ff634331b 
>   src/linux/perf.cpp 56ef3917ff2d5f761bda7721f8fccec4a3e14b7c 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0b171eeb53037f26b7e952830e88e59f1278e7c6 
>   src/tests/containerizer/perf_tests.cpp 
> 6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 
> 
> Diff: https://reviews.apache.org/r/37976/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Reply via email to