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



Thanks a lot for the patch! I've left some style comments, please address them, 
update the patch and we should be good to go.

Resources manipulation is versioned, which means we have almost identical 
implementation in "v1/resources.cpp". Could you please update that file as well?

In order to help people understand the impact of your patch, could you please 
run the test you've been running (the one you mentioned on Jira) without and 
with your patch and post both outputs here for comparison?


src/common/resources.cpp (lines 1476 - 1478)
<https://reviews.apache.org/r/49910/#comment207262>

    2 blank lines between function definitions, please!



src/common/resources.cpp (lines 1481 - 1486)
<https://reviews.apache.org/r/49910/#comment207267>

    Please indent by 2 spaces.



src/common/resources.cpp (lines 1486 - 1488)
<https://reviews.apache.org/r/49910/#comment207268>

    Let's add the default case as well (see e.g. how `Resource` is printed).



src/common/resources.cpp (lines 1490 - 1492)
<https://reviews.apache.org/r/49910/#comment207264>

    2 blank lines between functions definitions.


Than

- Alexander Rukletsov


On July 12, 2016, 6:15 a.m., Tim Harper wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49910/
> -----------------------------------------------------------
> 
> (Updated July 12, 2016, 6:15 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Joseph Wu.
> 
> 
> Bugs: MESOS-5824
>     https://issues.apache.org/jira/browse/MESOS-5824
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Output disk resource source information.
> 
> 
> Diffs
> -----
> 
>   src/common/resources.cpp cf07072 
> 
> Diff: https://reviews.apache.org/r/49910/diff/
> 
> 
> Testing
> -------
> 
> I ran `make check` in my OS X build environment. I had to disable SVN tests 
> because they didn't work, but I doubt this affected that feature set.
> 
> Resolves [MESOS-5824](https://issues.apache.org/jira/browse/MESOS-5824)
> 
> 
> Thanks,
> 
> Tim Harper
> 
>

Reply via email to