> On Oct. 21, 2015, 4:03 a.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/src/process.cpp, line 793
> > <https://reviews.apache.org/r/37999/diff/9/?file=1102348#file1102348line793>
> >
> >     Suggestion: break this up into two calls.
> >     
> >     removeAllAuthenticators()
> >     removeAuthenticator(<param>)
> 
> Alexander Rojas wrote:
>     I will let you decide this one with till, because he suggested the exact 
> opposite (which has also been suggested by other reviewers), in his Sept. 10, 
> 2015, 8:06 a.m. review (see above).

Till and I agreed on this:

clearAuthenticators();
removeAuthenticator(const &string realm);

Similar for removeEndpoint!

Why? For a single method with an option param you'd have to pick either 
singular or plural in the name and then it would be wrong in one of the two 
cases. Why clear and not removeAll? Because there is a lot of the former in the 
code base and none of the former.


- Bernd


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


On Nov. 4, 2015, 3:25 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2015, 3:25 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
>     https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduces the authenticator manager, which is a class which handles the 
> actual authentication procedure during the execution of 
> `ProcessManager::handle()` and it also takes care of the life cycle of 
> instances of http::Authenticator.
> 
> No tests are added at this point since no public API is generated, the goal 
> of this patch is to implement the manager and verify nothing breaks 
> afterwards. Authenticator manager tests proper come in a latter patch.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/include/process/http.hpp 
> 90c9be122ee0c402b806d70fc818e3c03b15101a 
>   3rdparty/libprocess/src/process.cpp 
> a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Reply via email to