mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Till Toenshoff <toensh...@me.com>
Subject Re: Review Request 59099: Enabled authorization for v1 API call UPDATE_MAINTENANCE_SCHEDULE.
Date Thu, 01 Jun 2017 21:59:09 GMT

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




src/master/http.cpp
Lines 4032-4034 (patched)
<https://reviews.apache.org/r/59099/#comment250062>

    Our formatting for the help texts seems to be all over the place -- I would like to avoid
making it even more inconsistent.
    
    What is the reason for the leading spaces in the wrapped lines?
    
    In my opinion, we can totally avoid that - simply killing all leading spaces in the wrapped
lines should do fine, no?
    
    Having leading spaces the way you now introduce seems to arguably have no value for the
user. When looking at such endpoint help while rendering with a proportionally spaced font,
which seems likely in a browser, will not show things in a matching fashion.



src/master/http.cpp
Lines 4096-4169 (original), 4113-4205 (patched)
<https://reviews.apache.org/r/59099/#comment250098>

    Can we please keep this a bit more tidy by not wrapping the lambdas?
    
    I would suggest to either chain those lambdas or use our good old continuation underscore
pattern and have the lambda/s be converted to functions. I am pretty sure it will become much
more readable then.



src/master/http.cpp
Lines 4119 (patched)
<https://reviews.apache.org/r/59099/#comment250063>

    Insert blank line please.



src/tests/api_tests.cpp
Lines 1205-1218 (patched)
<https://reviews.apache.org/r/59099/#comment250060>

    Use `AWAIT_EXPECT_RESPONSE_STATUS_EQ` instead please.



src/tests/api_tests.cpp
Lines 1193-1206 (original), 1225-1238 (patched)
<https://reviews.apache.org/r/59099/#comment250064>

    See above.


- Till Toenshoff


On June 1, 2017, 2:57 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59099/
> -----------------------------------------------------------
> 
> (Updated June 1, 2017, 2:57 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7415
>     https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enables the use of authorization for the `UPDATE_MAINTENANCE_SCHEDULE`
> v1 API call, using the ACL `UpdateMaintenanceSchedule` and the action
> of the same name as the API call.
> 
> It also updates the ApiTests to take into account the authorization
> case.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
>   src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 
>   src/tests/api_tests.cpp 91b3473452b8e65cab9f2e873837d64a0edf4b54 
> 
> 
> Diff: https://reviews.apache.org/r/59099/diff/5/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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