mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rukletsov" <ruklet...@gmail.com>
Subject Re: Review Request 39400: Quota: Implemented quota API.
Date Tue, 10 Nov 2015 22:59:59 GMT


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 140
> > <https://reviews.apache.org/r/39400/diff/5/?file=1109630#file1109630line140>
> >
> >     Maybe s/Introduce/Consider introducing/
> >     Are we sure we want to do this?

A very good point. I actually think each `TODO` which states something should have been converted
to code. The only reason to leave a `TODO` is because there is a certain level of uncertainty.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 894
> > <https://reviews.apache.org/r/39400/diff/5/?file=1109630#file1109630line894>
> >
> >     let's clarify `just updates the numbers`. Also missing a period.

Will do, please check if this helps.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 906
> > <https://reviews.apache.org/r/39400/diff/5/?file=1109630#file1109630line906>
> >
> >     A comment as to why we want to `allocate()` here would be useful.

Will do, please check if this helps.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 927
> > <https://reviews.apache.org/r/39400/diff/5/?file=1109630#file1109630line927>
> >
> >     A comment here would be helpful as well.

Will do, please check if this helps.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 225
> > <https://reviews.apache.org/r/39400/diff/5/?file=1109630#file1109630line225>
> >
> >     I think we can add an equivalent comment as above this for loop (not your fault):
> >     `// Update the allocation to this framework.`

Will do, please verify if this is what you would like to see here.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 330
> > <https://reviews.apache.org/r/39400/diff/5/?file=1109630#file1109630line330>
> >
> >     same as above. let's add a comment here.

Will do, please verify if this is what you would like to see here.


- Alexander


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


On Nov. 5, 2015, 6:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2015, 6:25 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, and Joseph
Wu.
> 
> 
> Bugs: MESOS-3718
>     https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp cfd937ba306273c24fb5337dfeb1a15e1545169b

>   src/master/allocator/mesos/hierarchical.cpp f4e4a123d3da0442e8b0b0ad14d1ee760752ba36

> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


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