> On Aug. 27, 2015, 6:04 a.m., Alexander Rukletsov wrote:
> > include/mesos/maintenance/maintenance.proto, line 56
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053335#file1053335line56>
> >
> >     I'm not sure `0` is a valid protobuf tag.
> 
> Joseph Wu wrote:
>     After double checking the docs... It is valid for enums: 
> https://developers.google.com/protocol-buffers/docs/proto#enum (But not valid 
> for non-enums.)
>     
>     For example: 
> https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L434

I would strongly recommend to use a start value of 1 instead of 0 because it's 
hard to tell the difference between an enum being unset to enum being set to 
the first value. This is especially true if the enum is going to be used as 
optional. Looks like it is used as a required field for now, but who knows.


- Vinod


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


On Aug. 27, 2015, 7:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 7:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, 
> Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, 
> Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to