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 46319: Added authorization to agents' `/monitor/statistics` endpoints.
Date Tue, 26 Apr 2016 16:51:53 GMT

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


Fix it, then Ship it!





src/slave/http.cpp (line 627)
<https://reviews.apache.org/r/46319/#comment194454>

    Indent the block by 2 spaces, not 4.



src/slave/http.cpp (line 635)
<https://reviews.apache.org/r/46319/#comment194455>

    Indent the block by 2 spaces, not 4.



src/slave/http.cpp (line 637)
<https://reviews.apache.org/r/46319/#comment194456>

    Indent the block by 2 spaces, not 4.



src/slave/http.cpp 
<https://reviews.apache.org/r/46319/#comment194459>

    You remove the `Future` here. I believe this is on purpose as it seems the right way to
pass the parameter (without future). Is it a bugfix or it doesn't influence the thrust?



src/tests/slave_authorization_tests.cpp (lines 172 - 173)
<https://reviews.apache.org/r/46319/#comment194457>

    I was about to ask why don't you use parameterized tests, but noticed you do it in the
next review. Any reason you don't want to fix it up in this patch?



src/tests/slave_authorization_tests.cpp (lines 198 - 200)
<https://reviews.apache.org/r/46319/#comment194458>

    Maybe kill this blank lines?


- Alexander Rukletsov


On April 26, 2016, 1:34 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46319/
> -----------------------------------------------------------
> 
> (Updated April 26, 2016, 1:34 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5164
>     https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 57b18882e30e44dcc40449b0e3be8ee970c45bc8 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46319/diff/
> 
> 
> Testing
> -------
> 
> make check (OS X, clang w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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