----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/#review56147 -----------------------------------------------------------
src/module/manager.cpp <https://reviews.apache.org/r/26529/#comment96471> Mind mentioning fixing this bug (and adding tests for it) in the RR description? We try to separate concerns / issues of RR's. Also, do you have the library name handy so you can give a bit more detailed error description? Imagine a long list of modules where a single one didn't have the module name set :-) src/tests/module_tests.cpp <https://reviews.apache.org/r/26529/#comment96470> Let's try to avoid macro constants - can we do static const char* or define a helper that does this for you? src/tests/module_tests.cpp <https://reviews.apache.org/r/26529/#comment96472> const strings if you don't intent to change them How about hoisting the 'LD_LIBRARY_PATH' string and define it once as a constant? - Niklas Nielsen On Oct. 10, 2014, 8:24 a.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26529/ > ----------------------------------------------------------- > > (Updated Oct. 10, 2014, 8:24 a.m.) > > > Review request for mesos and Niklas Nielsen. > > > Repository: mesos-git > > > Description > ------- > > Rename "path" in Modules protobuf to "file" since a "path" always referes to > an absolute or a relative path, whereas "file" may refer to a path or a file > name. If "file" contains a slash ("/"), it is considered an absolute or > relative path, otherwise if is considered a file name. For file name, > dlopen() automatically does a standard library path search to find the > absolute path. > > > Diffs > ----- > > include/mesos/module.hpp 733b1500ae3b833de4ce7585ba1b667ec164fdce > src/messages/messages.proto edf1e4ef83dfdd89f494cd215e629a304a8b84c1 > src/module/manager.cpp 72041c06b28191dea244a39ba99666e53198decc > src/tests/module_tests.cpp 6f9ee33f2f2d41dcc5bde9ef6326186f56e7c7fc > > Diff: https://reviews.apache.org/r/26529/diff/ > > > Testing > ------- > > Added tests for LD_LIBRARY_PATH and ran make check. > > > Thanks, > > Kapil Arya > >
