> On March 9, 2018, 11:16 p.m., Greg Mann wrote:
> > src/common/http.hpp
> > Lines 198 (patched)
> > <https://reviews.apache.org/r/65311/diff/6/?file=1972585#file1972585line198>
> >
> >     See the comment I left on https://reviews.apache.org/r/65313/ - I 
> > wonder if `action` really needs to be a template parameter here?

Strictly speaking it doesn't need to be a template, but then overrides like:

```c++
template <>
inline bool ObjectApprovers::approved<authorization::VIEW_ROLE>(
    const Resource& resource)
{
  // ...
}
```

Would have need to be dispatched to helper functions or handler in big ifs in 
`approved()`.

The end result would be, you just move the part hard to read from one side to 
the next.

My personal opinion is that the helper in the agent cleanup is more readable an 
a better price to pay than a gigantic switch here.


- Alexander


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


On March 8, 2018, 1:14 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65311/
> -----------------------------------------------------------
> 
> (Updated March 8, 2018, 1:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Greg Mann.
> 
> 
> Bugs: MESOS-8434
>     https://issues.apache.org/jira/browse/MESOS-8434
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduces the class `ObjectApprovers` which unifies the different
> mechanisms used in Mesos in order to perform authorization.
> 
> This new class will supersede the `Acceptor` interfaces and their
> logic.
> 
> Follow up patches will make use of this interface and remove
> deprecated code.
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp 750d3bfba1647624983108bdd23295a3b3091fe4 
>   src/common/http.cpp 8d3a4ad63bd74e0313082bad3d89c21fbf54f256 
> 
> 
> Diff: https://reviews.apache.org/r/65311/diff/6/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Reply via email to