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


Fix it, then Ship it!




Looks like we do not have quota information in `GET_ROLES`: 
https://github.com/apache/mesos/blob/master/include/mesos/v1/mesos.proto#L3628-L3634
Also, maybe we want to consider adding role info to the state?


src/common/http.hpp
Lines 219-221 (patched)
<https://reviews.apache.org/r/70877/#comment302949>

    Why the namespace here and below?



src/master/readonly_handler.cpp
Line 744 (original), 701 (patched)
<https://reviews.apache.org/r/70877/#comment302948>

    what is this for?



src/master/readonly_handler.cpp
Lines 766-770 (original), 721-736 (patched)
<https://reviews.apache.org/r/70877/#comment302947>

    Not very important, but I find putting shorter/simpler case on the top 
would improve readibilty.


- Meng Zhu


On June 18, 2019, 12:13 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70877/
> -----------------------------------------------------------
> 
> (Updated June 18, 2019, 12:13 p.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated master's /roles endpoint to use jsonify.
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp a0f9b67dca86f7c035035abed5d497aaf759bb10 
>   src/common/http.cpp dcf3cb0234fc81e155dae2267764c1dd7bed5201 
>   src/master/readonly_handler.cpp 66d6160c5e951e6c3a51eb02c31ead066c322202 
> 
> 
> Diff: https://reviews.apache.org/r/70877/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Reply via email to