----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49813/#review141788 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp (line 44) <https://reviews.apache.org/r/49813/#comment207165> I think for public interfaces, we use this style so that doxygen can pick up that. However, I don't think we should expose this class directly. Therefore, let's use `//` here. src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp (lines 85 - 88) <https://reviews.apache.org/r/49813/#comment207167> Can we add this method later when we actually filling the impl.? src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp (lines 91 - 94) <https://reviews.apache.org/r/49813/#comment207168> Ditto. Can we introduce this method when we actually fill the impl. src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp (lines 118 - 154) <https://reviews.apache.org/r/49813/#comment207169> Ditto. Let's don't introduce those helpers in this method. src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp (lines 158 - 160) <https://reviews.apache.org/r/49813/#comment207166> Let's use `//` here. src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 56 - 62) <https://reviews.apache.org/r/49813/#comment207170> Please don't introduce those helper yet in this patch. Ditto for all others. Only keep those virtual methods that's defined in the parent class. src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (lines 17 - 18) <https://reviews.apache.org/r/49813/#comment207172> I would probably further split this patch into two: 1) introduce the cgroups isolator 2) introduce the subsystem It'll be easier to review. src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (lines 74 - 75) <https://reviews.apache.org/r/49813/#comment207174> It's hard to tell why we need this function from this patch. That's the reason I suggested that we should adding helpers as we need those in the impl. - Jie Yu On July 9, 2016, 10:16 a.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49813/ > ----------------------------------------------------------- > > (Updated July 9, 2016, 10:16 a.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang. > > > Repository: mesos > > > Description > ------- > > Added stubs for the unified cgroups isolator. > > > Diffs > ----- > > src/CMakeLists.txt 760519c5568d22f778980b25c3dfedc3c8ef83b1 > src/Makefile.am 28dd15166937ed672f81be5a598df149b8ed4302 > src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION > 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/49813/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
