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




src/common/resources.cpp
Line 1378 (original), 1379-1384 (patched)
<https://reviews.apache.org/r/60017/#comment251513>

    Hm.. it's not obvious to me if this is the right thing to do, how much 
thought did you put into this? (can't tell given there's no comment)
    
    i.e. Should we be reducing the reservations down to a single one (and do we 
care about the distinction between static and dynamic for this stripped scalar 
quantity?) vs stripping metadata but preserving the stack.
    
    I think before we just cared about which role it was reserved to, not 
whether it was static or dynamic, so this is a change to the behavior.



src/common/resources.cpp
Lines 2055-2074 (original)
<https://reviews.apache.org/r/60017/#comment251509>

    This seems unrelated, can you pull it out of this change so that the diff 
is as clean as possible?



src/common/resources.cpp
Lines 2100-2115 (original), 2085-2101 (patched)
<https://reviews.apache.org/r/60017/#comment251514>

    Can you pull the printing out into a separate patch and show in the 
description what the format looks like?



src/common/resources_utils.cpp
Lines 42-44 (original), 42-44 (patched)
<https://reviews.apache.org/r/60017/#comment251510>

    Wow I didn't realize this function existed, looks really brittle. Hopefully 
whenever we write integration tests for new operations it doesn't work unless 
this is updated. :)



src/common/resources_utils.cpp
Lines 56-57 (original), 56-60 (patched)
<https://reviews.apache.org/r/60017/#comment251511>

    A little comment here would be great, took me a bit of time to understand 
what this was doing.



src/common/resources_utils.cpp
Lines 63-68 (original), 66-71 (patched)
<https://reviews.apache.org/r/60017/#comment251512>

    Not yours, but I wish there was a little comment here about why source is 
left in (took me a bit of time to figure that out)


- Benjamin Mahler


On June 13, 2017, 8:35 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60017/
> -----------------------------------------------------------
> 
> (Updated June 13, 2017, 8:35 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7655
>     https://issues.apache.org/jira/browse/MESOS-7655
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Resources: Updated utilities.
> 
> 
> Diffs
> -----
> 
>   src/common/resources.cpp f89f1aae7845f1e93dd5947de2e7a8e2bfce8bc4 
>   src/common/resources_utils.cpp c3088433d8c147b06dbef6f310fbe4059c3dc0ba 
>   src/v1/resources.cpp 24240eedc4b2747ca02998534437318c3396db4d 
> 
> 
> Diff: https://reviews.apache.org/r/60017/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Reply via email to