> On Dec. 1, 2015, 9:57 a.m., Joseph Wu wrote:
> > include/mesos/mesos.proto, line 653
> > <https://reviews.apache.org/r/40375/diff/5/?file=1146482#file1146482line653>
> >
> >     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.)

Yes; to keep the compatibility when serializing, I changed it to `optional`.


> On Dec. 1, 2015, 9:57 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 1402-1405
> > <https://reviews.apache.org/r/40375/diff/5/?file=1146484#file1146484line1402>
> >
> >     What are the backwards compatibility requirements for this?

I update the `operator<<` to return `{REV}` when `has_type()` return false.


> On Dec. 1, 2015, 9:57 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 98-110
> > <https://reviews.apache.org/r/40375/diff/5/?file=1146484#file1146484line98>
> >
> >     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.)

Agree, use `return left.type() == right.type()` for `operator==`.


> On Dec. 1, 2015, 9:57 a.m., Joseph Wu wrote:
> > include/mesos/mesos.proto, lines 644-650
> > <https://reviews.apache.org/r/40375/diff/5/?file=1146482#file1146482line644>
> >
> >     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.

Sure, I'll update comments accordingly.


- Klaus


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40375/#review108438
-----------------------------------------------------------


On Nov. 25, 2015, 2:16 p.m., Klaus Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40375/
> -----------------------------------------------------------
> 
> (Updated Nov. 25, 2015, 2: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
> 
>

Reply via email to