----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20015/#review40701 -----------------------------------------------------------
Looks great! 3rdparty/libprocess/include/process/metrics/counter.hpp <https://reviews.apache.org/r/20015/#comment73808> Ditto here, can you use None()? 3rdparty/libprocess/include/process/metrics/gauge.hpp <https://reviews.apache.org/r/20015/#comment73807> You should be able to s/Option<Duration>::none()/None()/ here? 3rdparty/libprocess/include/process/metrics/metric.hpp <https://reviews.apache.org/r/20015/#comment73806> Missing an include for Option and Duration? 3rdparty/libprocess/include/process/metrics/metric.hpp <https://reviews.apache.org/r/20015/#comment73803> Maybe a bit of documentation to describe what 'push' does? Would 'update' be clearer? 3rdparty/libprocess/include/process/metrics/metric.hpp <https://reviews.apache.org/r/20015/#comment73802> Why not stick the implementation of push() into Metric::push? I think our other 'Data' structs are pure structs, like Future::Data for example. 3rdparty/libprocess/include/process/metrics/metric.hpp <https://reviews.apache.org/r/20015/#comment73804> s/historyLock/lock/ for now? 3rdparty/libprocess/include/process/metrics/metric.hpp <https://reviews.apache.org/r/20015/#comment73805> Would it be better to use an Option<Owned<...>> instead of relying on NULL? 3rdparty/libprocess/src/tests/metrics_tests.cpp <https://reviews.apache.org/r/20015/#comment73810> Unused? 3rdparty/libprocess/src/tests/metrics_tests.cpp <https://reviews.apache.org/r/20015/#comment73809> Kill the whitespace here? It's generally good practice to look over your diff once it's up on review board to catch things before sending it out (although whitespace linting would work here too). - Ben Mahler On April 16, 2014, 10:51 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20015/ > ----------------------------------------------------------- > > (Updated April 16, 2014, 10:51 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-1036 > https://issues.apache.org/jira/browse/MESOS-1036 > > > Repository: mesos-git > > > Description > ------- > > see summary. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/metrics/counter.hpp > f4774ada4dbe3fa18b5a8b204f97f59ca015b3c1 > 3rdparty/libprocess/include/process/metrics/gauge.hpp > 4f5c1086ac3553319431283165c5451df1a0ee3f > 3rdparty/libprocess/include/process/metrics/metric.hpp > ea64f699fd9ec38745d84c7523133709827f96db > 3rdparty/libprocess/src/tests/metrics_tests.cpp > 0cc9f4bcbbb03ac3a9a2d57f64b944443fcb94bb > > Diff: https://reviews.apache.org/r/20015/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Dominic Hamon > >
