----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21279/#review43020 -----------------------------------------------------------
Ship it! Looks good, would like to see some consistency around post-increment vs. pre-increment for metrics and counters. src/master/master.cpp <https://reviews.apache.org/r/21279/#comment77003> No period here src/master/master.cpp <https://reviews.apache.org/r/21279/#comment77006> Now we're pre-incrementing 'count' here, post-incrementing 'count' below, pre-incrementing some 'metrics', post-incrementing some 'metrics' (in master and slave). We should stick with one technique, and I think the preference in Mesos has been to post-increment for "readability" which is arguable but is the current state (avoiding the thought of optimization here as it's definitely premature and the compiler will often take care of it). We're not consistent across the code by any means, and my personal preference is to only use post-increment when necessary, however the easiest change here would be to just post-increment in this change. - Ben Mahler On May 14, 2014, 8:50 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21279/ > ----------------------------------------------------------- > > (Updated May 14, 2014, 8:50 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-1332 > https://issues.apache.org/jira/browse/MESOS-1332 > > > Repository: mesos-git > > > Description > ------- > > see summary > > > Diffs > ----- > > src/master/master.hpp 12111cf7b183438f540bb0aad17e50e3f367558a > src/master/master.cpp 2f0e9027c5bea8066ac13996451d4905a422336e > src/tests/master_tests.cpp dcda0c79a6693bc3c0b69b4c9b148f9608342738 > > Diff: https://reviews.apache.org/r/21279/diff/ > > > Testing > ------- > > make check > > ran locally and manually verified. > > > Thanks, > > Dominic Hamon > >
