----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27493/#review60124 -----------------------------------------------------------
Ship it! Looks great! Besides renaming the process to "crammd5_authenticatee", I could only find a couple of nits. src/authentication/cram_md5/authenticatee.hpp <https://reviews.apache.org/r/27493/#comment101455> Why the change in wrapping? I think it would still fit the other way, and if you are bringing credential down to the next line, it might be better if you give the UPID its own line too. src/authentication/cram_md5/authenticatee.hpp <https://reviews.apache.org/r/27493/#comment101456> s/authenticatee/crammd5_authenticatee/ src/authentication/cram_md5/authenticatee.hpp <https://reviews.apache.org/r/27493/#comment101457> "requires a credential" (singular)? src/examples/test_authentication_module.cpp <https://reviews.apache.org/r/27493/#comment101458> Maybe this file should be "test_authentication_modules.cpp" (plural) since it contains multiple modules? - Adam B On Nov. 5, 2014, 8:24 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27493/ > ----------------------------------------------------------- > > (Updated Nov. 5, 2014, 8:24 a.m.) > > > Review request for mesos, Adam B, Kapil Arya, and Vinod Kone. > > > Bugs: MESOS-2001 > https://issues.apache.org/jira/browse/MESOS-2001 > > > Repository: mesos-git > > > Description > ------- > > see summary. > > > Diffs > ----- > > src/Makefile.am 2d72a70 > src/authentication/authenticatee.hpp PRE-CREATION > src/authentication/cram_md5/authenticatee.hpp PRE-CREATION > src/examples/test_authentication_module.cpp PRE-CREATION > src/examples/test_authenticator_module.cpp PRE-CREATION > src/module/authenticatee.hpp PRE-CREATION > src/module/manager.cpp 7a6c884 > > Diff: https://reviews.apache.org/r/27493/diff/ > > > Testing > ------- > > make check > > Note: All three CRAM-MD5 Authenticatee patches need to get applied before > running make check! > > > Thanks, > > Till Toenshoff > >
