Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On July 28, 2016, 6:14 p.m., Jiang Yan Xu wrote: > > src/tests/resources_tests.cpp, line 2727 > > > > > > I apologize for my typo in the previous review, I was going to suggest > > a `count` test because it's not

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review144059 --- Fix it, then Ship it! I'll commit with the following minor adju

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated July 30, 2016, 6:59 a.m.) Review request for mesos, Benjamin Mahler, J

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated July 29, 2016, 7:09 p.m.) Review request for mesos, Benjamin Mahler, J

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On July 29, 2016, 1:14 a.m., Jiang Yan Xu wrote: > > src/common/resources.cpp, lines 848-850 > > > > > > Just trying to see if the comments can be made more concise: > > > > ``` > > Assuming the wrappe

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On July 20, 2016, 2:36 a.m., Klaus Ma wrote: > > src/common/resources.cpp, lines 308-316 > > > > > > I think we can check name,type, role firstly, then check SharedInfo. Although I updated it based on your comment

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review143997 --- This is done before the you rebased but it should be straightforwa

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated July 28, 2016, 10:42 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On July 20, 2016, 2:36 a.m., Klaus Ma wrote: > > include/mesos/resources.hpp, line 124 > > > > > > Seems `Option` is not necessary. sharedCount is None when it is is regular non-shared resource, and is an integer

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated July 21, 2016, 12:12 a.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review142892 --- include/mesos/resources.hpp (line 84)

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On July 19, 2016, 5:03 p.m., Jiang Yan Xu wrote: > > It would be helpful to add a test just for contains. i.e., to test how > > `contains` works with nonshared resources and how it works with simple > > shared resources (could be constructed from createDiskResources with no > > arithmetic op

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated July 19, 2016, 10:51 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On July 19, 2016, 10:03 a.m., Jiang Yan Xu wrote: > > It would be helpful to add a test just for contains. i.e., to test how > > `contains` works with nonshared resources and how it works with simple > > shared resources (could be constructed from createDiskResources with no > > arithmetic o

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review142652 --- It would be helpful to add a test just for contains. i.e., to test

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated July 18, 2016, 2:29 p.m.) Review request for mesos, Benjamin Mahler, J

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated July 17, 2016, 8:44 p.m.) Review request for mesos, Benjamin Mahler, J

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On July 8, 2016, 4:30 p.m., Jiang Yan Xu wrote: > > src/tests/resources_tests.cpp, line 2563 > > > > > > I suggest we don't reuse variables, perhaps even use `const`. > > > > ``` > > const Resources s

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated July 8, 2016, 11:03 p.m.) Review request for mesos, Benjamin Mahler, J

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review141176 --- Code LGTM. :) Comments mainly on tests. include/mesos/resources.

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review137070 --- include/mesos/resources.hpp (line 121)

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated June 13, 2016, 7:18 a.m.) Review request for mesos, Benjamin Mahler, J

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On June 10, 2016, 12:10 a.m., Jiang Yan Xu wrote: > > I feel good about the overall structure of this change with this revision. > > I hope I have captured most things so if we do another pass after it should > > mostly be relatively minor readability/reusablity/documentation/cleanups > > et

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On June 9, 2016, 5:10 p.m., Jiang Yan Xu wrote: > > I feel good about the overall structure of this change with this revision. > > I hope I have captured most things so if we do another pass after it should > > mostly be relatively minor readability/reusablity/documentation/cleanups > > etc.

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review136417 --- I feel good about the overall structure of this change with this r

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On May 16, 2016, 6:01 p.m., Jiang Yan Xu wrote: > > include/mesos/resources.hpp, line 190 > > > > > > This is supposed to be hidden in private right? Done as a part of the refactor. Few of the comments below have

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated May 22, 2016, 7:08 a.m.) Review request for mesos, Benjamin Mahler, Jo

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review131398 --- Haven't looked at tests but given the anticipated change in the ne

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On April 9, 2016, 8:48 a.m., Guangya Liu wrote: > > src/common/resources.cpp, lines 296-304 > > > > > > Why not return false if the resource type is not `SCALAR` because only > > `SCALAR` was supportted for now R

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On April 18, 2016, 6:09 a.m., Jiang Yan Xu wrote: > > include/mesos/resources.hpp, line 56 > > > > > > Let's use a `class` just like Resources. As a `Resource` wrapper > > eventually most methods for single resourc

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated April 29, 2016, 12:15 a.m.) Review request for mesos, Ben Mahler, Jori

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review128700 --- Partial review, mainly in resources.hpp|cpp. Some high level note

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

> On April 9, 2016, 9:02 a.m., Guangya Liu wrote: > > You may want to rebase this patch for dynamic_reservation_framework.cpp as > > well. Yes, done. - Anindya --- This is an automatically generated e-mail. To reply, visit: https://rev

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- (Updated April 12, 2016, 5 a.m.) Review request for mesos and Jiang Yan Xu. C

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review127969 --- You may want to rebase this patch for dynamic_reservation_framewor

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review127964 --- include/mesos/resources.hpp (line 64)

Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/ --- Review request for mesos and Jiang Yan Xu. Bugs: MESOS-4892 https://issues.