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

Ship it!


Looks good, just some minor items below that I'll address before committing. 
Notably there was a redundant if condition left after the change to use 
.get()'s Option.


3rdparty/libprocess/include/process/authenticator.hpp (line 91)
<https://reviews.apache.org/r/38094/#comment173167>

    Not necessary, but should we put the virtual here?



3rdparty/libprocess/src/authenticator.cpp (line 22)
<https://reviews.apache.org/r/38094/#comment173160>

    Where did this come from?



3rdparty/libprocess/src/authenticator.cpp (lines 34 - 36)
<https://reviews.apache.org/r/38094/#comment173165>

    We're already in the http namespace, did you need these?



3rdparty/libprocess/src/authenticator.cpp (lines 71 - 73)
<https://reviews.apache.org/r/38094/#comment173161>

    This case is redundant with your credentials.isNone() check below?



3rdparty/libprocess/src/authenticator.cpp (line 76)
<https://reviews.apache.org/r/38094/#comment173163>

    There is a newline here between the variable declaration and the 'if' 
statement, but there isn't in two cases below, then the last case has a 
newline. How about we just use a newline in each case here?



3rdparty/libprocess/src/authenticator.cpp (line 82)
<https://reviews.apache.org/r/38094/#comment173166>

    We should probably error out here if the size is not exactly 2, much like 
you did below for the credentials.



3rdparty/libprocess/src/authenticator.cpp (line 96)
<https://reviews.apache.org/r/38094/#comment173164>

    Seems a bit odd to put a body here, but not in the other cases, no?


- Ben Mahler


On Jan. 4, 2016, 11:26 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2016, 11:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
>     https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am 6749fd40e672c3626f6605756bd787a878271034 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> ec7b4aa90bf7024469bf9774e01e46c9f7fd094f 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Reply via email to