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



include/mesos/maintenance/maintenance.proto (lines 31 - 37)
<https://reviews.apache.org/r/36571/#comment152258>

    IIUC, here you group a bunch of machines based on time intervals. Why this 
grouping is necessary? We can simplify the design significantly and reduce the 
number of protobufs if we represent schedule by a set of maintenance events, 
each per machine, which may occasionally overlap in time.
    
    See my comment in https://reviews.apache.org/r/37314 with a proposal for a 
joined "maintenance event" message.



include/mesos/maintenance/maintenance.proto (line 56)
<https://reviews.apache.org/r/36571/#comment152241>

    I'm not sure `0` is a valid protobuf tag.



include/mesos/mesos.proto (line 123)
<https://reviews.apache.org/r/36571/#comment152265>

    Minor nit: capitalize NOTE.



include/mesos/mesos.proto (line 136)
<https://reviews.apache.org/r/36571/#comment152238>

    Why do we need this message? I've checked several following patches and 
haven't seen any use cases. Shall I look further up review chain?



src/Makefile.am (line 913)
<https://reviews.apache.org/r/36571/#comment152237>

    Looks like we maintain alphabetical order here.



src/master/registry.proto (line 45)
<https://reviews.apache.org/r/36571/#comment152239>

    Suffix `Info` is used mostly to avoid collisions with non-proto classes, 
therefore feel free to omit `_info` in the name here.


- Alexander Rukletsov


On Aug. 26, 2015, 6:35 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 6:35 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 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to