> On May 23, 2016, 6:12 a.m., Guangya Liu wrote: > > src/slave/containerizer/mesos/isolator.hpp, lines 122-123 > > <https://reviews.apache.org/r/47706/diff/1/?file=1391124#file1391124line122> > > > > I think it is better to keep those two lines as comments for `status()`
I would normally agree, except that this comment really doesn't say anything that the name of the function doesn't already convey. > On May 23, 2016, 6:12 a.m., Guangya Liu wrote: > > src/slave/containerizer/mesos/isolators/xfs/disk.cpp, lines 291-299 > > <https://reviews.apache.org/r/47706/diff/1/?file=1391145#file1391145line291> > > > > Lost to update the file in disk.hpp? Yep, looked like I missed one. Thanks. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47706/#review134308 ----------------------------------------------------------- On May 23, 2016, 12:44 a.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47706/ > ----------------------------------------------------------- > > (Updated May 23, 2016, 12:44 a.m.) > > > Review request for mesos, Benjamin Mahler and Jie Yu. > > > Bugs: MESOS-5435 > https://issues.apache.org/jira/browse/MESOS-5435 > > > Repository: mesos > > > Description > ------- > > Previously all of the virtual functions in `mesos::slave::Isolator` were > pure virtual (expect status()). For many isolators, however, it > doesn't make sense to implement all of these virtual functions. > Currently, each isolator has to provide its own default implementation > of these functions even if they aren't really relying on them. This > adds unnecessary extra code to many isolators that don't need them. > > In addition to the changes to `mesos::slave::Isolator`, this commit > also provides default implementations for all of the corresponding > virtual functions in `MesosIsolatorProcess`. This way, classes that > inherit from `MesosIsolatorProcess` get the same benefit of code reuse > as those that inherit form `mesos::slave::Isolator`. > > > Diffs > ----- > > include/mesos/slave/isolator.hpp 4be8c2bb409052e2e07138483408209384f41e23 > src/slave/containerizer/mesos/isolator.hpp > bacd86af42d16cb7c9b6622dfb298dcaa7007b75 > src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp > 502204650192d5ea44aa631eac8eb37e051843f0 > src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp > 8f81cb79c10261670efc9eaa8614751854f53806 > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp > d7592cf49f4e4c5f3fc6a3244d9b922d4eb70a9f > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp > 54ded996c879c41163cbf7e9c1bb7ae6807c8801 > src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp > 65e731886b9e5cac07ae3ad6398faf8f50de5650 > src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp > 336ae0a4c6e2ad519f7913cd819f2ddea82c5cec > src/slave/containerizer/mesos/isolators/docker/runtime.hpp > 90179119ef297855091dad3fe969aa79810bf209 > src/slave/containerizer/mesos/isolators/docker/runtime.cpp > a12220a2693271fc192ab9165b176b1f3d18b9ce > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp > 01777d4a656657e591593631fda49787f7d9fe55 > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp > 92fce1cbdcd0d2cbd68b02ab02c5c224c42d67a4 > src/slave/containerizer/mesos/isolators/filesystem/linux.hpp > c99f33a77e4db5407cc26361a2f253b00e91f5b5 > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > 5edbce95cc9eb4d6d22f9ab1528902cc745780af > src/slave/containerizer/mesos/isolators/filesystem/posix.hpp > c6cea98e16f2bdea2da0220c235468080bbcd17b > src/slave/containerizer/mesos/isolators/filesystem/posix.cpp > 01c0ad6dbb6d509e62e769365586b3d23dcb240d > src/slave/containerizer/mesos/isolators/filesystem/shared.hpp > dd0481cd7b5aaa8e198160d9d604090041033605 > src/slave/containerizer/mesos/isolators/filesystem/shared.cpp > 9ae4d937c27286f18026c47b61b2f8fcff6e74a6 > src/slave/containerizer/mesos/isolators/namespaces/pid.hpp > c8f389d36138681795641088d3ef686def0a4e64 > src/slave/containerizer/mesos/isolators/namespaces/pid.cpp > 5970d22e7198236d22c55de6153f465ed5f5fd7a > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp > a06bb89a7a79a62949a48274df806f9f95da09e7 > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > dae369aadb940150aa806b28d9269e3d88cf57ed > src/slave/containerizer/mesos/isolators/xfs/disk.cpp > 2f65f0a072911d3384684609828ec6cb6a27d19a > > Diff: https://reviews.apache.org/r/47706/diff/ > > > Testing > ------- > > make -j check > > > Thanks, > > Kevin Klues > >
