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




src/master/authorization.cpp
Lines 111 (patched)
<https://reviews.apache.org/r/71864/#comment306852>

    This comment isn't helpful, can you describe what this does? 
    
    It seems a lot easier to understand this function if this just returned 
action objects instead of pushing into an existing vector (and push being in 
the name?). Further, wouldn't this be even simpler if it just took a singular 
resource and the caller loops?
    
    Ditto for the one below.
    
    If the additonal push behavior is helpful, maybe just lamdba them in 
reserve since that's where the code uses them several times within complex 
logic?



src/master/authorization.cpp
Lines 239-240 (patched)
<https://reviews.apache.org/r/71864/#comment306850>

    Ditto the broader question about validation errors from the previous review.



src/master/authorization.cpp
Lines 242 (patched)
<https://reviews.apache.org/r/71864/#comment306851>

    be sure to look at the indents when you self-review before publishing :)


- Benjamin Mahler


On Dec. 3, 2019, 5:55 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71864/
> -----------------------------------------------------------
> 
> (Updated Dec. 3, 2019, 5:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benjamin Mahler.
> 
> 
> Bugs: MESOS-10023 and MESOS-10056
>     https://issues.apache.org/jira/browse/MESOS-10023
>     https://issues.apache.org/jira/browse/MESOS-10056
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Moved creating authorization Object out of `Master::authorize*reserve`.
> 
> 
> Diffs
> -----
> 
>   src/master/authorization.hpp PRE-CREATION 
>   src/master/authorization.cpp PRE-CREATION 
>   src/master/http.cpp e03655863ea2d4e4464b3d14b359de3d7f059778 
>   src/master/master.hpp 93630421d58e6fd26566e81a23cd910957795665 
>   src/master/master.cpp 14b90a5e276df055bb8a570331f27cab200c9869 
> 
> 
> Diff: https://reviews.apache.org/r/71864/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Reply via email to