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 41550: [5/5] Quota Authorization: Added tests for quota removal authorization.
Date Wed, 06 Jan 2016 11:40:22 GMT


> On Jan. 5, 2016, 6: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
> 
> Jan Schlicht wrote:
>     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 Schlicht wrote:
>     Talked with my Shepherd about this. Let's remove the boilerplate code and add the
comments. If these change in your review, you'd need to change them here as well, okay?

Sure!


- Alexander


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


On Jan. 6, 2016, 10:53 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41550/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2016, 10:53 a.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 81f0386d072d0176cb28eb229b274e424004f54c 
> 
> 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