----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30131/#review69646 -----------------------------------------------------------
Ship it! Modulo the Filter comment, we should consider making that consistent with the rest of our code (T, BarT, FooT, ...), or at the very least add a NOTE as to why we nest all the subclasses. include/mesos/resources.hpp <https://reviews.apache.org/r/30131/#comment114389> It's a bit unusual to define all the child classes in the base class, the general pattern seems to be: ``` T FooT BarT ... ``` Any reason to stray from that in this case, but not in other cases? Keeping it inside Resources sounds good for now, but it might make it inconsistent for others to add Filters (since this is a public interface), maybe add a TODO to pull it out, possibly inside a resources::filters namespace? include/mesos/resources.hpp <https://reviews.apache.org/r/30131/#comment114384> Do you want this implicit? Looks like you're being explicit in the .cpp code when using this. include/mesos/resources.hpp <https://reviews.apache.org/r/30131/#comment114386> Not yours, but should this be an Option? src/tests/resources_tests.cpp <https://reviews.apache.org/r/30131/#comment114391> volume1 and volume2? Like you did below? - Ben Mahler On Jan. 26, 2015, 6:21 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30131/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2015, 6:21 p.m.) > > > Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > Used persistent volumes consistently in the code base. (previously, we used > persistent disk). > > > Diffs > ----- > > include/mesos/resources.hpp 7935e7f9bfe66d1900594dcdcb800c4593a3940f > src/common/resources.cpp 214e441fb86aa0c094c28ed5801089051468137b > src/master/drf_sorter.cpp 967c8aa00606445785836c3e3825a6f467746d33 > src/master/master.cpp bda8fda9bc2e52ccc1d75e2541e4604989515e13 > src/messages/messages.proto 5bd075e9bfe17be334294b5e4c573eb0233030be > src/slave/containerizer/isolators/posix/disk.cpp > bb1779fd644a33527068868a45cf9c19d7732727 > src/tests/resource_offers_tests.cpp > 4395dd248bd410a2a2887e39a5f9a99d41e2735b > src/tests/resources_tests.cpp b7c1ddfda952f2d6b7f82e90cfe7b6b0aafbc36a > src/tests/sorter_tests.cpp 56e5714c2ab97d0ac81d29e1acb1fbec15471489 > > Diff: https://reviews.apache.org/r/30131/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
