----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45084/#review127895 -----------------------------------------------------------
Does this patch compile on its own? I'd like each patch to be 'atomic' so that we can commit some of them if they look good. src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (lines 24 - 25) <https://reviews.apache.org/r/45084/#comment191252> Instead of that, can you just include <mesos/slave/isolator.hpp>? src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 32) <https://reviews.apache.org/r/45084/#comment191253> Why do you need this header? src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 46) <https://reviews.apache.org/r/45084/#comment191264> Could you please put `flags` as the first parameter? src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 48) <https://reviews.apache.org/r/45084/#comment191262> Why return a raw pointer, instead of an Owned pointer here? src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 53) <https://reviews.apache.org/r/45084/#comment191263> Could you please put `flags` as the first parameter? src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 57) <https://reviews.apache.org/r/45084/#comment191310> `virtual std::string name() const = 0;` We typically do not return const string. src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 28) <https://reviews.apache.org/r/45084/#comment191255> Please use explicit using clauses here. src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 68 - 83) <https://reviews.apache.org/r/45084/#comment191257> Since you're just adding stubs in this patch, can you introduce such logics later? It's hard to review if you stick in logics like this without any context. - Jie Yu On April 7, 2016, 10:36 a.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45084/ > ----------------------------------------------------------- > > (Updated April 7, 2016, 10:36 a.m.) > > > Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and > Kevin Klues. > > > Bugs: MESOS-5039 > https://issues.apache.org/jira/browse/MESOS-5039 > > > Repository: mesos > > > Description > ------- > > Add `Subsystem` abstraction for cgroups. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/45084/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
