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




src/master/registry.proto
Lines 116 (patched)
<https://reviews.apache.org/r/62353/#comment261998>

    indentation?



src/resource_provider/registrar.hpp
Lines 131 (patched)
<https://reviews.apache.org/r/62353/#comment261999>

    tailing `{` should be in a new line



src/resource_provider/registrar.hpp
Lines 133 (patched)
<https://reviews.apache.org/r/62353/#comment262001>

    No need for `explicit` because this is a two arguments constructor



src/resource_provider/registrar.hpp
Lines 144 (patched)
<https://reviews.apache.org/r/62353/#comment262002>

    nits: no need for tailing underscore to be consistent with other files.



src/resource_provider/registrar.cpp
Lines 382 (patched)
<https://reviews.apache.org/r/62353/#comment262023>

    Ditto. No need for `explicit`



src/resource_provider/registrar.hpp
Lines 134 (patched)
<https://reviews.apache.org/r/62353/#comment262163>

    Move the tailing `{` to the next line.



src/resource_provider/registrar.hpp
Lines 147 (patched)
<https://reviews.apache.org/r/62353/#comment262164>

    Ditto. Remove tailing underscore



src/resource_provider/registrar.cpp
Lines 372-376 (patched)
<https://reviews.apache.org/r/62353/#comment262165>

    This is definitely weird to me. I think we can get rid of the dependency of taking an internal `internal::Registry& registry` by doing the following:
    
    1) Move `AdaptedOperation` to master registrar. I would call it `UpdateResourceProviderRegistryOperation`.
    
    2) `recover()` probably does not need to return `Registry`. If we get rid of that, we probably don't need to expose the internal Registry.


- Jie Yu


On Sept. 19, 2017, 4:54 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62353/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2017, 4:54 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds an implementation of the resource provider registrar
> backed by the master's registrar. With that it becomes possible to
> persist resource provider manager state in a single master registrar,
> but use the narrowly defined resource provider registry.
> 
> 
> Diffs
> -----
> 
>   src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/tests/resource_provider_manager_tests.cpp 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/62353/diff/2/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>