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




src/Makefile.am
Lines 1015 (patched)
<https://reviews.apache.org/r/64012/#comment269964>

    s/reconfiguration/compatibility/



src/slave/flags.cpp
Lines 462 (patched)
<https://reviews.apache.org/r/64012/#comment269932>

    s/configuration_compatibility/reconfiguration_compatibility/



src/slave/reconfiguration.hpp
Lines 28 (patched)
<https://reviews.apache.org/r/64012/#comment269965>

    put this in `compatibility` namespace.



src/slave/reconfiguration.hpp
Lines 37 (patched)
<https://reviews.apache.org/r/64012/#comment269966>

    `equal`



src/slave/reconfiguration.hpp
Lines 63 (patched)
<https://reviews.apache.org/r/64012/#comment269967>

    `additive`



src/slave/reconfiguration.hpp
Lines 67 (patched)
<https://reviews.apache.org/r/64012/#comment269963>

    singe blank line.



src/slave/reconfiguration.cpp
Lines 51 (patched)
<https://reviews.apache.org/r/64012/#comment269951>

    period at the end.
    
    `Resource`
    `Attribute`



src/slave/reconfiguration.cpp
Lines 91 (patched)
<https://reviews.apache.org/r/64012/#comment269952>

    "Port" in the next line



src/slave/reconfiguration.cpp
Lines 99-100 (patched)
<https://reviews.apache.org/r/64012/#comment269954>

    `previous.domain`
    `current.domain`



src/slave/reconfiguration.cpp
Lines 103 (patched)
<https://reviews.apache.org/r/64012/#comment269955>

    `resources.size()`



src/slave/reconfiguration.cpp
Lines 108-115 (patched)
<https://reviews.apache.org/r/64012/#comment269960>

    pull this above the `switch` statement.



src/slave/reconfiguration.cpp
Lines 171-178 (patched)
<https://reviews.apache.org/r/64012/#comment269962>

    ditto. pull this up `switch`.



src/slave/slave.cpp
Lines 6126-6137 (patched)
<https://reviews.apache.org/r/64012/#comment269949>

    inline.



src/slave/slave.cpp
Lines 6300 (patched)
<https://reviews.apache.org/r/64012/#comment269934>

    s/our/the/
    
    s/it will turn/it turns/



src/slave/slave.cpp
Lines 6307 (patched)
<https://reviews.apache.org/r/64012/#comment269936>

    s/checkConfigurationCompatibility/compatible/



src/slave/slave.cpp
Lines 6314 (patched)
<https://reviews.apache.org/r/64012/#comment269946>

    remove the bool.



src/slave/slave.cpp
Lines 6316 (patched)
<https://reviews.apache.org/r/64012/#comment269939>

    s/we keep/the slave keeps/



src/slave/slave.cpp
Lines 6318 (patched)
<https://reviews.apache.org/r/64012/#comment269937>

    can you inline this?



src/slave/slave.cpp
Lines 6328-6330 (patched)
<https://reviews.apache.org/r/64012/#comment269942>

    kill this.



src/slave/slave.cpp
Line 6255 (original), 6336 (patched)
<https://reviews.apache.org/r/64012/#comment269943>

    s/mismatch/incompatibility (defined as `equal` in Mesos 1.4 and could be 
`equal` or `additive` in Mesos 1.5)/



src/slave/slave.cpp
Line 6260 (original), 6341 (patched)
<https://reviews.apache.org/r/64012/#comment269947>

    just return here.



src/slave/slave.cpp
Line 6286 (original), 6358-6361 (patched)
<https://reviews.apache.org/r/64012/#comment269948>

    put this in an else block with a comment on when this block is reached.



src/tests/reconfiguration_tests.cpp
Lines 2 (patched)
<https://reviews.apache.org/r/64012/#comment269976>

    Can you add more end to end tests in `slave_recovery_tests.cpp` ?



src/tests/reconfiguration_tests.cpp
Lines 30 (patched)
<https://reviews.apache.org/r/64012/#comment269969>

    s/make/create/



src/tests/reconfiguration_tests.cpp
Lines 48 (patched)
<https://reviews.apache.org/r/64012/#comment269968>

    2 blank lines.



src/tests/reconfiguration_tests.cpp
Lines 49-50 (patched)
<https://reviews.apache.org/r/64012/#comment269973>

    Can you write the correct comment?



src/tests/reconfiguration_tests.cpp
Lines 66 (patched)
<https://reviews.apache.org/r/64012/#comment269971>

    2 blank lines.



src/tests/reconfiguration_tests.cpp
Lines 85 (patched)
<https://reviews.apache.org/r/64012/#comment269974>

    1 blank line.



src/tests/reconfiguration_tests.cpp
Lines 95 (patched)
<https://reviews.apache.org/r/64012/#comment269975>

    Can you make sure to test all cases?


- Vinod Kone


On Nov. 28, 2017, 12:51 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64012/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2017, 12:51 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added new --configuration_compatibility slave flag and implementation.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 49dec55ced32945ad0414c32eb4e00247f4b54f2 
>   src/slave/flags.hpp 0c02b49c054ef2597298aefe0997beacfc9efeb8 
>   src/slave/flags.cpp 0eeecdc6023443a99c8e8fe29c2f7bf791c0a36e 
>   src/slave/reconfiguration.hpp PRE-CREATION 
>   src/slave/reconfiguration.cpp PRE-CREATION 
>   src/slave/slave.hpp 40442f25ae1b223380e11f8602e6256c9203ef47 
>   src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
>   src/tests/reconfiguration_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64012/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Reply via email to