mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Schlicht <...@mesosphere.io>
Subject Re: Review Request 46784: Added authorization of the '/flags' endpoint.
Date Mon, 09 May 2016 12:27:08 GMT


> On April 29, 2016, 11:20 a.m., Benjamin Bannier wrote:
> > src/master/http.cpp, line 868
> > <https://reviews.apache.org/r/46784/diff/1/?file=1364766#file1364766line868>
> >
> >     Could you make this capture list explicit (`[this, request]`)?
> 
> Jan Schlicht wrote:
>     The style guide recommends to prefer default capture by value, that's why I'm doing
it here. The case here is different from the one in `slave/http.cpp`. There we explicitly
captured by value to be sure not to capture `this`, because that would lead to problems there.
We don't have this problem here, hence the default capture by value.
> 
> Benjamin Bannier wrote:
>     Note that `[this, request]` *does* capture by value. I am just asking you to be explicit
about what you capture instead of pulling in everything from the current scope.
> 
> Jan Schlicht wrote:
>     As does `[=]`, and that's preferred by the style guide.
> 
> Benjamin Bannier wrote:
>     Hm, I read that part of the style guide differently, but well. I believe we value
explicit over implicit, but even in this file there seems to be no preference either way,
so feel free to drop this issue.

Seems that I'm the only one that read the style guide that way, see the discussion with @alexr
below. I'll change it to explicit capture.


- Jan


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


On May 3, 2016, 2:42 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46784/
> -----------------------------------------------------------
> 
> (Updated May 3, 2016, 2:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5297
>     https://issues.apache.org/jira/browse/MESOS-5297
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Access to the '/flags' endpoint is authorized using
> the 'GET_ENDPOINT_WITH_PATH' action.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 4985f24b70a00116caa4bd0335ea51e55d878d19 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/tests/master_authorization_tests.cpp 804b39a269c09df9f6c0bbdf6f8b53921ac09ce8 
> 
> Diff: https://reviews.apache.org/r/46784/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


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