----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review73359 -----------------------------------------------------------
The code looks great! Given that the tests are quite large. Can you split this patch and I'll give a shipit for the code. For tests, I need to do a pass next week. Thanks! THis is great, Chi! src/linux/cgroups.hpp <https://reviews.apache.org/r/30545/#comment119660> Any reason this is not a static method of pressure::Listener? src/linux/cgroups.hpp <https://reviews.apache.org/r/30545/#comment119661> Well, you don't need this friend declaration if 'create' is a static member funciton, right? src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment119664> Look like only one place is using this constant. Sugguest killing it. src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment119663> I think we indent 2 spaces for switch 'case'. src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment119662> I think here should be able to do: ``` : coutner_(0), error(None()), process(new event::Listener( hierarchy, cgroup, "memory.pressure_level", stringify(level))) {} ``` src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment119668> Remove the tailing period. src/linux/cgroups.cpp <https://reviews.apache.org/r/30545/#comment119669> We don't use std:: prefix in cpp files. Please remove them (do a sweep in this file). - Jie Yu On Feb. 20, 2015, 9:37 p.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30545/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2015, 9:37 p.m.) > > > Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. > > > Bugs: MESOS-2136 > https://issues.apache.org/jira/browse/MESOS-2136 > > > Repository: mesos > > > Description > ------- > > cgroups: added support to listen on memory pressures. > > > Diffs > ----- > > src/Makefile.am d372404d84c9ac0bc49da7407ad366701b9586a6 > src/linux/cgroups.hpp e07772fec11b9bb73a44c54326f24d7ee1e8a64b > src/linux/cgroups.cpp a307e27c5840270e28cced2d5aeb90c6679bff1d > src/tests/cgroups_tests.cpp 9d50a47c939f5f136c85fdcc555459512c6f2259 > src/tests/memory_test_helper.hpp PRE-CREATION > src/tests/memory_test_helper.cpp PRE-CREATION > src/tests/memory_test_helper_main.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/30545/diff/ > > > Testing > ------- > > > Thanks, > > Chi Zhang > >
