----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21013/#review42027 -----------------------------------------------------------
Hey Tim, Here is a style review - I will take it for a spin today. 3rdparty/libprocess/3rdparty/Makefile.am <https://reviews.apache.org/r/21013/#comment75754> s/dynamiclibrary_tests/dynamic_library_tests/ 3rdparty/libprocess/3rdparty/Makefile.am <https://reviews.apache.org/r/21013/#comment75753> Kill trailing spaces 3rdparty/libprocess/3rdparty/stout/Makefile.am <https://reviews.apache.org/r/21013/#comment75755> s/dynamiclibrary.hpp/dynamic_library.hpp/ 3rdparty/libprocess/3rdparty/stout/Makefile.am <https://reviews.apache.org/r/21013/#comment75756> s/dynamiclibrary_tests.cpp/dynamic_library_tests.cpp/ 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75773> Rename the file to dynamic_library.hpp 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75770> Kill newline 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75757> spaces between assignment. s/m_handle=0;/m_handle = 0;/ s/m_librarypath="NOTLOADED";/m_librarypath = "NOTLOADED";/ Also, consider using initializer list instead. 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75758> s/std::string &/std::string&/ 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75759> One line: } else { 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75772> You never return false, how about using Try<Nothing> instead (and do it in the other functions too). 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75760> Same as mentioned before. One line. 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75761> Spaces between assignment 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75771> s/std::string &/std::string&/ s/void *&/void*& / 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75762> Please wrap according to style guide: return Error( "Error looking up symbol '" + symbolname + "' in '" + m_librarypath + "' : " + dlerror()) 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75763> s/void * /void* / 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp <https://reviews.apache.org/r/21013/#comment75764> We don't use "m_" for member fields. 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp <https://reviews.apache.org/r/21013/#comment75769> Missing license block? 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp <https://reviews.apache.org/r/21013/#comment75765> Comments end with period. 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp <https://reviews.apache.org/r/21013/#comment75766> Kill newline 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp <https://reviews.apache.org/r/21013/#comment75767> s/checkreturn/result/? 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp <https://reviews.apache.org/r/21013/#comment75768> s/void * /void* / s/0/NULL/ - Niklas Nielsen On May 2, 2014, 9:44 a.m., Timothy St. Clair wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21013/ > ----------------------------------------------------------- > > (Updated May 2, 2014, 9:44 a.m.) > > > Review request for mesos, Benjamin Hindman and Niklas Nielsen. > > > Bugs: MESOS-1224 > https://issues.apache.org/jira/browse/MESOS-1224 > > > Repository: mesos-git > > > Description > ------- > > Addition of DynamicLibrary class to stout, to enable future plugins inside of > mesos. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/Makefile.am 8ccf0ef > 3rdparty/libprocess/3rdparty/stout/Makefile.am aa1d0a5 > 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp > PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp > PRE-CREATION > 3rdparty/libprocess/configure.ac fc5e2ad > > Diff: https://reviews.apache.org/r/21013/diff/ > > > Testing > ------- > > New test has been added and ran 'make check' around Mac and Linux > > > Thanks, > > Timothy St. Clair > >
