----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26857/#review58909 -----------------------------------------------------------
We added a new module kind but didn't add any new tests. Wouldn't it be better to move the authenticator example to the patch where we add the test case or the other way around? src/authentication/authenticator.hpp <https://reviews.apache.org/r/26857/#comment100079> Shouldn't this be __AUTHENTICATION_AUTHENTICATOR_HPP__ src/authentication/authenticator.hpp <https://reviews.apache.org/r/26857/#comment100081> Shouldn't we also explicitly include <process/pid.h> (although it is being included indirectly by process/future.hpp). src/authentication/authenticator.hpp <https://reviews.apache.org/r/26857/#comment100080> Same as above. src/examples/test_authenticator_module.cpp <https://reviews.apache.org/r/26857/#comment100098> The file module/authenticator.hpp is not part of this patch. Shouldn't it be removed from https://reviews.apache.org/r/26859/diff/# and added here? Also, alphabetize please. src/examples/test_authenticator_module.cpp <https://reviews.apache.org/r/26857/#comment100100> The indentation is odd here. - Kapil Arya On Oct. 28, 2014, 8:43 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26857/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2014, 8:43 a.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone. > > > Bugs: MESOS-1889 > https://issues.apache.org/jira/browse/MESOS-1889 > > > Repository: mesos-git > > > Description > ------- > > Introducing the CRAM-MD5 SASL authenticator module based on the former > sasl/authenticator. > > The former sasl/authenticator.hpp still remains as part of libmesos but now > is located at authentication/cram_md5. > > > Diffs > ----- > > src/Makefile.am f177d87 > src/authentication/authenticator.hpp PRE-CREATION > src/authentication/cram_md5/authenticator.hpp PRE-CREATION > src/examples/test_authenticator_module.cpp PRE-CREATION > src/module/manager.cpp 7a6c884 > > Diff: https://reviews.apache.org/r/26857/diff/ > > > Testing > ------- > > make check > > NOTE all three CRAM-MD5 authenticator module related RRs need to get applied > before running make check. > > > Thanks, > > Till Toenshoff > >
