----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review53198 -----------------------------------------------------------
Thanks for doing this! A few higher level comments: (1) We have strings::join in stout. Have you considered implementing this TLineHelper as a generic Joiner in stout/strings.hpp? Seems like it belongs there instead of a newline joiner in this file. Let's also be sure to not expose it in the higher level namespace. (2) We tend to leverage support/post-reviews.py to break changes apart, for example, the addition of the variadic template and the breaking up of help into a .cpp file could have been done in two separate reviews to speed up the review / commit process. We could get the latter change committed quickly! As much as possible, breaking changes apart into a chain of commits will make your life easier. :) (3) From the discussion on MESOS-750, it seems that gcc 4.4 is the minimum supported compiler version. FWICT, this means that within mesos we can certainly assume variadic templates. My hunch is that we don't want to continue writing both C++11/non-C++11 code paths in stout/libprocess due to the complexity and the maintenance burden, but that's a decision we should seek benh's input on. For now, the #ifdef is certainly a safe way to go. - Ben Mahler On Aug. 28, 2014, 4:24 p.m., Patrick Reilly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25079/ > ----------------------------------------------------------- > > (Updated Aug. 28, 2014, 4:24 p.m.) > > > Review request for mesos, Adam B and Benjamin Hindman. > > > Bugs: MESOS-1734 > https://issues.apache.org/jira/browse/MESOS-1734 > > > Repository: mesos-git > > > Description > ------- > > Reduce compile time: - replacing a macro expansion with a variadic template > - moving implementation from help.hpp to help.cpp > > > Diffs > ----- > > 3rdparty/libprocess/Makefile.am edbe54b > 3rdparty/libprocess/include/process/help.hpp 4333b5b > 3rdparty/libprocess/src/help.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/25079/diff/ > > > Testing > ------- > > Ran "make check". > > > Thanks, > > Patrick Reilly > >
