----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42504/#review115275 -----------------------------------------------------------
src/common/resources.cpp (lines 918 - 919) <https://reviews.apache.org/r/42504/#comment176227> We should update the comments here. src/common/resources.cpp (line 926) <https://reviews.apache.org/r/42504/#comment176232> No need for the 'has_disk()' check before it's already checked above. THe following looks more clean to me. ``` if (stripped.disk().has_source()) { ... } else { ... } ``` src/common/resources.cpp (lines 961 - 966) <https://reviews.apache.org/r/42504/#comment176233> Do you need to update this as well? src/tests/persistent_volume_tests.cpp (lines 91 - 106) <https://reviews.apache.org/r/42504/#comment176253> Instead of overwriting CreateSlaveFlags() which is a little implicit, can we add a SlaveResources() method to return the slave's resources to be more explicit. ``` Resources getDiskResources(const Megabytes& mb) { ... } slaveFlags.resources = getDiskResources(...); ``` src/tests/persistent_volume_tests.cpp (lines 576 - 580) <https://reviews.apache.org/r/42504/#comment176262> Wondering if we should introduce an overload of 'createPersistentVolume' that takes resource, id and containerPath. The might be able to simply your subsequent patch on tests. - Jie Yu On Jan. 19, 2016, 3:51 p.m., Joris Van Remoortere wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42504/ > ----------------------------------------------------------- > > (Updated Jan. 19, 2016, 3:51 p.m.) > > > Review request for mesos, Jie Yu and Michael Park. > > > Repository: mesos > > > Description > ------- > > Refactored persistent volume tests to use common slave resources. > > > Diffs > ----- > > src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a > src/common/resources_utils.cpp d64db54a175299bf7c32ecbeeb1ce9a7afb1c88a > src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac > src/tests/persistent_volume_tests.cpp > 7acf7ab29d64d891f3288f8042d267dcc82a32e9 > src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 > src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc > > Diff: https://reviews.apache.org/r/42504/diff/ > > > Testing > ------- > > > Thanks, > > Joris Van Remoortere > >
