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


Fix it, then Ship it!





src/resource_provider/manager.cpp
Lines 75 (patched)
<https://reviews.apache.org/r/65470/#comment276496>

    If this section is sorted like includes, this should go right after the 
first `process::http` section just below.



src/resource_provider/manager.cpp
Lines 200-201 (original), 218-219 (patched)
<https://reviews.apache.org/r/65470/#comment276509>

    Not yours, but we should pull these onto the line of the last parameter 
like elsewhere.



src/resource_provider/manager.cpp
Lines 772 (patched)
<https://reviews.apache.org/r/65470/#comment276510>

    At some point we'll move the prefix to a constant somewhere.



src/tests/resource_provider_manager_tests.cpp
Lines 1322-1323 (patched)
<https://reviews.apache.org/r/65470/#comment276511>

    `StartMaster` will automatically pass default `masterFlags` if none are 
given. Let's remove them here since we do not need them in the test.



src/tests/resource_provider_manager_tests.cpp
Lines 1352 (patched)
<https://reviews.apache.org/r/65470/#comment276512>

    `s/subscribed1/subscribed/g`
    
    Here and below.



src/tests/resource_provider_manager_tests.cpp
Lines 1365 (patched)
<https://reviews.apache.org/r/65470/#comment276513>

    `map`'s `operator[]` is not `const` which makes it unclear whether the test 
unknowingly modifies the test data. I'd suggest to make `snapshot` `const` and 
then use `at` here. We should also add an assertion to the values contain that 
key just before attempting to access it.


- Benjamin Bannier


On Feb. 2, 2018, 1:27 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65470/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2018, 1:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Greg Mann.
> 
> 
> Bugs: MESOS-8527
>     https://issues.apache.org/jira/browse/MESOS-8527
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added metrics for showing number of subscribed resource providers.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/manager.cpp 5d064bff76d5f18d85e6c050adf9edbc26107c07 
>   src/tests/resource_provider_manager_tests.cpp 
> 2944b066f8bfcbe328469940eb0e17b9b2809f63 
> 
> 
> Diff: https://reviews.apache.org/r/65470/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to