----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/#review94156 -----------------------------------------------------------
Ship it! src/linux/perf.cpp (lines 292 - 294) <https://reviews.apache.org/r/37045/#comment148689> The style checker doesn't catch this, but this isn't one of the styles for wrapping function calls in the style guide, can you wrap at the open paren? src/linux/perf.cpp (line 298) <https://reviews.apache.org/r/37045/#comment148681> No need for this comment IMO src/linux/perf.cpp (line 301) <https://reviews.apache.org/r/37045/#comment148690> Can you print the failure if the future is failed? Note that you can CHECK that it isn't discarded. src/linux/perf.cpp (lines 306 - 307) <https://reviews.apache.org/r/37045/#comment148692> Note that we don't use periods when composing log messages, so: Failed to collect perf statistics: perf exited with status <status> Note that : is used for composition, so when we say which status we wouldn't use a : src/linux/perf.cpp (line 307) <https://reviews.apache.org/r/37045/#comment148693> Can you use WSTRINGIFY instead here? Note that we should not be using WEXITSTATUS when WIFEXITED is not true. WSTRINGIFY will take care of this for you. src/linux/perf.cpp (line 312) <https://reviews.apache.org/r/37045/#comment148694> Let's CHECK that this is not discarded as well, before we call .get src/linux/perf.cpp (line 314) <https://reviews.apache.org/r/37045/#comment148680> Notice that we don't put a space before the : when composing failure messages, seems minor but inconsistent log message formatting is a huge pain. - Ben Mahler On Aug. 4, 2015, 11:59 p.m., Paul Brett wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37045/ > ----------------------------------------------------------- > > (Updated Aug. 4, 2015, 11:59 p.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 > >
