----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40375/#review108438 -----------------------------------------------------------
include/mesos/mesos.proto (lines 644 - 650) <https://reviews.apache.org/r/40375/#comment167897> Can you expand a little on these enum descriptions? Ideally, these should include: * A brief definition of each resource type. * Which components control the resource and briefly how they exert control. include/mesos/mesos.proto (line 653) <https://reviews.apache.org/r/40375/#comment167899> Can you note that this is effectively required? i.e. Internally, we will ensure a `type` exists in all valid resources. (This may simplify some other code.) src/common/resources.cpp (lines 98 - 110) <https://reviews.apache.org/r/40375/#comment167898> Note: When you specify a default, it is safe to call `protobuf.type()` without checking if it exists first. See: https://developers.google.com/protocol-buffers/docs/proto#optional So minimally, you could just `return left.type() == right.type();`. (But if you do this, you need to comment about the default.) src/common/resources.cpp (lines 1402 - 1405) <https://reviews.apache.org/r/40375/#comment167901> What are the backwards compatibility requirements for this? src/tests/resources_tests.cpp (line 804) <https://reviews.apache.org/r/40375/#comment167902> This shouldn't pass after your modifications. - Joseph Wu On Nov. 24, 2015, 10:16 p.m., Klaus Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40375/ > ----------------------------------------------------------- > > (Updated Nov. 24, 2015, 10:16 p.m.) > > > Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van > Remoortere. > > > Bugs: MESOS-3888 > https://issues.apache.org/jira/browse/MESOS-3888 > > > Repository: mesos > > > Description > ------- > > MESOS-3888: We need to distinguish revocable resource for usage slack and > allocation slack. > > > Diffs > ----- > > include/mesos/mesos.proto 27971fe > include/mesos/v1/mesos.proto 9acefd5 > src/common/resources.cpp b4abf54 > src/tests/resources_tests.cpp dbd39cd > src/v1/resources.cpp 8488c31 > > Diff: https://reviews.apache.org/r/40375/diff/ > > > Testing > ------- > > make && make check > > > Thanks, > > Klaus Ma > >
