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

Ship it!


I added the comments to the test you added, but some of them apply to 
`AuthenticatedSlaveText` as well. Let's fix those as well :)


src/tests/credentials_tests.cpp (line 119)
<https://reviews.apache.org/r/39098/#comment160047>

    Let's rename this `s/flags/masterFlags/` and move it to just above 
`flags.load(values, true)` below.



src/tests/credentials_tests.cpp (line 121)
<https://reviews.apache.org/r/39098/#comment160046>

    `path::join` returns a temporary string right?
    In which case: `s/const string &/string/`.



src/tests/credentials_tests.cpp (line 145)
<https://reviews.apache.org/r/39098/#comment160050>

    Add newline after



src/tests/credentials_tests.cpp (line 147)
<https://reviews.apache.org/r/39098/#comment160048>

    Let's just use `s/flags.credentials.get().value/path/` here?



src/tests/credentials_tests.cpp (line 158)
<https://reviews.apache.org/r/39098/#comment160049>

    Remove newline.


- Michael Park


On Oct. 13, 2015, 7:53 a.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2015, 7:53 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3560
>     https://issues.apache.org/jira/browse/MESOS-3560
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When decoding the JSON credential file into the Credential protobuf object, 
> the 'secret' which is in 'bytes' is mapped into base64 string automatically 
> by protobuf from JSON. This creates an unintended behavior, forcing users to 
> encode in base64 their secret when wanting to pass a JSON file to the 
> --credentials flag.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/examples/java/TestExceptionFramework.java 78720b0 
>   src/examples/java/TestFramework.java aad94c0 
>   src/tests/credentials_tests.cpp ced27c4 
> 
> Diff: https://reviews.apache.org/r/39098/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Reply via email to