> On Jan. 22, 2015, 12:43 a.m., Ben Mahler wrote: > > include/mesos/resources.hpp, line 157 > > <https://reviews.apache.org/r/30130/diff/1/?file=828560#file828560line157> > > > > How are you going to use this one? It seems like we will normally have > > a `RepeatedPtrField<Offer::Operation>`, do you want that instead? > > > > Or do you want a templatized version? > > > > ``` > > template <typename Iterable> > > Try<Resources> apply(const Iterable<Offer::Operation>& operations) > > const; > > ```
I go with the templatized version. > On Jan. 22, 2015, 12:43 a.m., Ben Mahler wrote: > > src/common/resources.cpp, lines 629-631 > > <https://reviews.apache.org/r/30130/diff/1/?file=828561#file828561line629> > > > > How about the following to be a bit more explicit and consistent: > > > > ``` > > if (!volume.has_disk()) { > > return Error("Invalid CREATE Operation: Expecting 'disk' to be > > present"); > > } else if (!volume.disk().has_persistence)) { > > return Error("Invalid CREATE Operation: Expecting 'persistence' to be > > present"); > > } > > ``` > > > > Could you use this pattern througout this method for all Error returns? > > > > ``` > > "Invalid X Operation: <reason>" > > ``` > > > > Some of the existing errors don't include the operation type. Done. > On Jan. 22, 2015, 12:43 a.m., Ben Mahler wrote: > > src/common/resources.cpp, lines 661-663 > > <https://reviews.apache.org/r/30130/diff/1/?file=828561#file828561line661> > > > > Ditto here for splitting these two cases apart. Done. > On Jan. 22, 2015, 12:43 a.m., Ben Mahler wrote: > > src/master/hierarchical_allocator_process.hpp, lines 603-604 > > <https://reviews.apache.org/r/30130/diff/1/?file=828563#file828563line603> > > > > What happened here? Mind leaving as is to make the diff smaller? Reverted. > On Jan. 22, 2015, 12:43 a.m., Ben Mahler wrote: > > src/master/allocator.hpp, line 202 > > <https://reviews.apache.org/r/30130/diff/1/?file=828562#file828562line202> > > > > How about 'updateAllocation' now that we avoid using "transformation"? I am renamed transform to update in sorter. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30130/#review69089 ----------------------------------------------------------- On Jan. 21, 2015, 6:11 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30130/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2015, 6:11 p.m.) > > > Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > Killed Resources::Transformation in favor of using Offer::Operation. > > This patch is based on and is a replacement for > https://reviews.apache.org/r/29744/ > > > Diffs > ----- > > include/mesos/resources.hpp 7935e7f9bfe66d1900594dcdcb800c4593a3940f > src/common/resources.cpp 214e441fb86aa0c094c28ed5801089051468137b > src/master/allocator.hpp 224569a55eb15d9e7c63f2b571a1f9ae8cd4b6e6 > src/master/hierarchical_allocator_process.hpp > ccd37b4348c785ece890257f805777acbaf8d58c > src/tests/hierarchical_allocator_tests.cpp > 7c051232f2c70691a7789e0f3e94f0e4cba26f18 > src/tests/mesos.hpp 591134bab31da0a1cc224e332666d05f320d0f87 > src/tests/resources_tests.cpp b7c1ddfda952f2d6b7f82e90cfe7b6b0aafbc36a > src/tests/sorter_tests.cpp 56e5714c2ab97d0ac81d29e1acb1fbec15471489 > > Diff: https://reviews.apache.org/r/30130/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
