----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28089/#review61667 -----------------------------------------------------------
include/mesos/resources.hpp <https://reviews.apache.org/r/28089/#comment103426> I would suggest keeping the behavior symmetric to `std::vector` here. I'm guessing this was added due to the annoyance around initialization. The same issue existed for `std::vector` in `C++98` but it's been fixed with `std::initializer_list` constructors in `C++11`. We can implement one for `Resources` as well. ```cpp // C++98 std::vector<int> v; v.push_back(1); v.push_back(2); // C++11 std::vector<int> v = {1, 2}; ``` For `Resources`, it would look like: ```cpp Resources(std::initializer_list<Resource> resources) { foreach (const Resource& resource, resources) { *this += resource; } } ``` Then we can do the following transformation: ```cpp // Before Resource cpus1 = Resources::parse("cpus", "1", "role1").get(); Resources r1; r1 += cpus1; // After Resources r1 = { Resources::parse("cpus", "1", "role1").get() }; ``` src/common/resources.cpp <https://reviews.apache.org/r/28089/#comment103419> How about using `combinable` here? `if (!combinable(left, right)) { return false; }` src/common/resources.cpp <https://reviews.apache.org/r/28089/#comment103420> Ditto here, use `combinable`? src/common/resources.cpp <https://reviews.apache.org/r/28089/#comment103424> This variable isn't necessary right? src/common/resources.cpp <https://reviews.apache.org/r/28089/#comment103423> Hm, just curious. Do we need the `Clear` here even if we're now doing `CopyFrom`? - Michael Park On Nov. 15, 2014, 6:56 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28089/ > ----------------------------------------------------------- > > (Updated Nov. 15, 2014, 6:56 a.m.) > > > Review request for mesos, Ben Mahler and Vinod Kone. > > > Bugs: MESOS-1974 > https://issues.apache.org/jira/browse/MESOS-1974 > > > Repository: mesos-git > > > Description > ------- > > Hide operators for Resource object. Split "matches" to handle future first > class infos. > > > Diffs > ----- > > include/mesos/resources.hpp 0e37170 > src/common/resources.cpp e9a0c85 > src/tests/resources_tests.cpp 3e50889 > > Diff: https://reviews.apache.org/r/28089/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
