mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 42471: Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.
Date Tue, 19 Jan 2016 22:50:54 GMT

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


Do we want to expose == and != for DiskInfo::Source yet? We don't even have == and != exposed
for DiskInfo yet (same for ReservationInfo, etc.). Maybe make them file local helpers in src/common/resources.cpp
for now.


include/mesos/type_utils.hpp (line 29)
<https://reviews.apache.org/r/42471/#comment176201>

    Do you need this?



include/mesos/type_utils.hpp (lines 64 - 74)
<https://reviews.apache.org/r/42471/#comment176190>

    Not yours. I wish we could sort them according to lexical order as we did before.



include/mesos/type_utils.hpp (lines 167 - 175)
<https://reviews.apache.org/r/42471/#comment176200>

    Can you move this above `operator==(const SlaveID&`



include/mesos/type_utils.hpp (line 171)
<https://reviews.apache.org/r/42471/#comment176202>

    Can you move non trivial impl. to common/type_utils.cpp?



include/mesos/type_utils.hpp (lines 179 - 187)
<https://reviews.apache.org/r/42471/#comment176203>

    Ditto. Move non trivial impl. to cpp file.



include/mesos/type_utils.hpp (lines 242 - 255)
<https://reviews.apache.org/r/42471/#comment176197>

    Not yours, can you put these above SlaveID.



include/mesos/v1/mesos.hpp (line 30)
<https://reviews.apache.org/r/42471/#comment176204>

    No you need this?



include/mesos/v1/values.hpp (line 22)
<https://reviews.apache.org/r/42471/#comment176205>

    Why this change?



src/common/resources.cpp (lines 93 - 99)
<https://reviews.apache.org/r/42471/#comment176208>

    I prefer to check 'source' first. Can we move this up above the persistence checks.



src/v1/resources.cpp (lines 93 - 99)
<https://reviews.apache.org/r/42471/#comment176209>

    Ditto.


- Jie Yu


On Jan. 18, 2016, 9:23 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42471/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4380
>     https://issues.apache.org/jira/browse/MESOS-4380
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
>   include/mesos/v1/mesos.hpp 961042d8e4944a475076b829966020d62175d726 
>   include/mesos/v1/values.hpp 03a19ee44b3d7f801f7ca2a61f1d42d51e0c144b 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42471/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message