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