----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28966/#review64986 -----------------------------------------------------------
src/slave/containerizer/isolators/network/port_mapping.hpp <https://reviews.apache.org/r/28966/#comment107836> this should come after gauge.hpp (alphabetical) src/slave/containerizer/isolators/network/port_mapping.hpp <https://reviews.apache.org/r/28966/#comment107837> explicit for constructors with single args please src/slave/containerizer/isolators/network/port_mapping.hpp <https://reviews.apache.org/r/28966/#comment107838> move this below the other methods (it's nice to have the constructor first) and add a comment above "// Gauge methods." src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/28966/#comment107839> instead of this, you can pass in the PID<PortMappingIsolatorProcess> directly to the contstructor and initialize the Metrics object with (self()). Just check that order of construction is correct and 'self()' will return the right thing ;) src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/28966/#comment107840> how expensive is this going to be? Can we determine the number of filters without constructing the port ranges, or cache the number of filters we've added? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/28966/#comment107841> bring this up a line, break after the = if you have to. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/28966/#comment107842> EXPECT_GT and maybe even EXPECT_EQ is better so you exercise the actual logic. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/28966/#comment107843> EXPECT_EQ - Dominic Hamon On Dec. 12, 2014, 2:45 p.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28966/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2014, 2:45 p.m.) > > > Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. > > > Repository: mesos-git > > > Description > ------- > > also added a test. > > > Diffs > ----- > > src/slave/containerizer/isolators/network/port_mapping.hpp f1e2352 > src/slave/containerizer/isolators/network/port_mapping.cpp c6fff21 > src/tests/port_mapping_tests.cpp eb82993 > > Diff: https://reviews.apache.org/r/28966/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Chi Zhang > >
