mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jan Schlicht" <...@mesosphere.io>
Subject Re: Review Request 41550: [5/5] Quota Authorization: Added tests for quota removal authorization.
Date Wed, 06 Jan 2016 10:15:38 GMT


> On Jan. 5, 2016, 7:51 p.m., Alexander Rukletsov wrote:
> > src/tests/master_quota_tests.cpp, lines 1308-1318
> > <https://reviews.apache.org/r/41550/diff/3/?file=1182024#file1182024line1308>
> >
> >     You don't really need an agent here, you can set the force flag and remove this
boilerplate code. Check the cleanup here: https://reviews.apache.org/r/41939

This is great! But I have some concerns that are more review related. I would need to add
the same comments as you do in the cleanup. As your code is currently being reviewed, these
comments may change and I would need to change them as well, or vice versa. Therefore I think
it would be better to only fix this in your cleanup and leave it like that for now. Do you
agree? I'm mostly concerned about the comments explaining why the force flag is set for the
HTTP requests.


- Jan


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


On Jan. 5, 2016, 4:13 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41550/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-4083
>     https://issues.apache.org/jira/browse/MESOS-4083
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Quota Authorization: Added tests for quota removal authorization.
> 
> 
> Diffs
> -----
> 
>   src/tests/authorization_tests.cpp 1d11a02032c142455debd9c78d4ee4cc6297a350 
>   src/tests/master_quota_tests.cpp 2f1bc3ae6a370e466f7cea9b597f51d7eccb1b33 
> 
> Diff: https://reviews.apache.org/r/41550/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


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