----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38627/#review102245 -----------------------------------------------------------
Would it be possible to add a few unit tests, also to show usage patterns? especially given the absence of any documentation, it's kinda difficult to figure out how is this "intended to work" and, without tests, whether it works at all. src/module/manager.hpp (line 102) <https://reviews.apache.org/r/38627/#comment159819> would it be possible to have proper, full javadoc for this method (as well as the other one)? these will be used by external module developers, so having them well-documented would be really great. src/tests/module.hpp (lines 76 - 77) <https://reviews.apache.org/r/38627/#comment159821> same comment here - it would be great to have fully-documented, properly-formatted javadoc here, also explaining what could go into `parameters` how will this affect creating the module, etc. while this may all appear obvious to you, it may not be so to external developers using this code to launch/test modules. thanks! src/tests/module.hpp (line 80) <https://reviews.apache.org/r/38627/#comment159823> (I understand this may not be your choice, but...) I find `N` as an identifier is a poor choice because (a) it's entirely non-obvious *what* is it and (b) I'd expect it anyway to be an `int` of some sort. Could you please consider renaming it to be something more expressive? thanks! - Marco Massenzio On Oct. 2, 2015, 8:11 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38627/ > ----------------------------------------------------------- > > (Updated Oct. 2, 2015, 8:11 p.m.) > > > Review request for mesos, Adam B, Bernd Mathiske, Niklas Nielsen, and Till > Toenshoff. > > > Bugs: MESOS-3232 > https://issues.apache.org/jira/browse/MESOS-3232 > > > Repository: mesos > > > Description > ------- > > Allows developers to provide their own parameters when loading modules > instead of using the ones provided by the user when loading Mesos. This helps > to deal with default modules (those used when the user doesn't provide any), > and for testing of the modules. > > > Diffs > ----- > > src/module/manager.hpp 302eb409fb8ef53b9cef8d2ecbe7b7f452b095ef > src/tests/module.hpp 0820978441aede18dae6d1701433bff705b8c3c2 > > Diff: https://reviews.apache.org/r/38627/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
