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

Ship it!


Thanks for the test!


src/common/protobuf_utils.cpp (lines 178 - 187)
<https://reviews.apache.org/r/40036/#comment164130>

    You don't need the 'if' with the way you've structured your loop, I assume 
it's just here from copy paste?



src/common/protobuf_utils.cpp (line 182)
<https://reviews.apache.org/r/40036/#comment164131>

    Consider:
    
    ```
    for (auto status = task.statuses().rbegin(); status != 
task.statuses().rend(); ++status)
    ```
    
    Ideally we could do:
    
    ```
    foreach (const TaskStatus& status, reverse(task.statuses()))
    ```
    
    If you want to avoid the indexing math.



src/common/protobuf_utils.cpp (line 190)
<https://reviews.apache.org/r/40036/#comment164129>

    two newlines here :)



src/tests/master_tests.cpp (line 3369)
<https://reviews.apache.org/r/40036/#comment164134>

    The convention here would be `status2` or something like 
`reconciliationStatus`.



src/tests/master_tests.cpp (lines 3373 - 3377)
<https://reviews.apache.org/r/40036/#comment164136>

    You can avoid the vector here by passing in an initializer list into your 
reconcileTasks call.



src/tests/master_tests.cpp (line 3386)
<https://reviews.apache.org/r/40036/#comment164137>

    We now have the `->` operator for Future, could you do a sweep in the code 
you've introduced to replace `.get().` with `->`? Thanks!!



src/tests/master_tests.cpp (line 3394)
<https://reviews.apache.org/r/40036/#comment164135>

    Ditto here.


- Ben Mahler


On Nov. 6, 2015, 7:53 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40036/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2015, 7:53 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3845
>     https://issues.apache.org/jira/browse/MESOS-3845
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Along the lines of health checks, the container_status should also be sent
> inside reconciliation updates.
> 
> 
> Diffs
> -----
> 
>   src/common/protobuf_utils.hpp 44a2b1d3bab227294f5c5854acd87b7bd75ef589 
>   src/common/protobuf_utils.cpp 1e795dcc90b2225ad7b0a124dbdcdabdc9d2e6aa 
>   src/master/master.cpp cbae27e7a4059a72bc69e152ec8adaf4ef725965 
>   src/tests/master_tests.cpp 856440559de7d58be5bcf2ab6be911b5c67001cd 
> 
> Diff: https://reviews.apache.org/r/40036/diff/
> 
> 
> Testing
> -------
> 
> `make check` with updated `MasterTest.TaskStatusContainerStatus` test.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Reply via email to