Re: Review Request 25079: Replaced macro expansion with variadic template
On Sept. 12, 2014, 7:03 p.m., Ben Mahler wrote: 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. Cody Maloney wrote: (1) There are actually a number string joiners throughout the codebase, and lots of unnecessary constant string - std::string - appending. Things like the newline adder add a lot of extra compile time while not actually giving less typing than people inserting '\n' by hand (On top of the runtime behavior doing less unnecessary computation)... (3) I've worked with variadic templates on gcc 4.6 - 4.9, and each until about 4.7 has slight changes in behavior / bugfixes. We really need a bot moving into C++11 land to guarantee that all the compilers we say we support work. Ben Mahler wrote: (1) Good point, we can make strings::join variadic for C++11 and it can do the job for us in help.hpp, to avoid further proliferation of string joiners. :) (3) Ok, let's keep the #ifdef for now! I have submitted r25789 and will modify this patch to use the strings::join provided from there. - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review53198 --- 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
Re: Review Request 25079: Replaced macro expansion with variadic template
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review53196 --- Ship it! This is awesome! I will commit shortly: I fixed a few minor style nits (removing a duplicate comment and adding newlines). - Niklas Nielsen On Aug. 28, 2014, 9:24 a.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 28, 2014, 9:24 a.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
Re: Review Request 25079: Replaced macro expansion with variadic template
--- 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
Re: Review Request 25079: Replaced macro expansion with variadic template
On Sept. 12, 2014, 7:03 p.m., Ben Mahler wrote: 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. (1) There are actually a number string joiners throughout the codebase, and lots of unnecessary constant string - std::string - appending. Things like the newline adder add a lot of extra compile time while not actually giving less typing than people inserting '\n' by hand (On top of the runtime behavior doing less unnecessary computation)... (3) I've worked with variadic templates on gcc 4.6 - 4.9, and each until about 4.7 has slight changes in behavior / bugfixes. We really need a bot moving into C++11 land to guarantee that all the compilers we say we support work. - Cody --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review53198 --- 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
Re: Review Request 25079: Replaced macro expansion with variadic template
On Sept. 12, 2014, 7:03 p.m., Ben Mahler wrote: 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. Cody Maloney wrote: (1) There are actually a number string joiners throughout the codebase, and lots of unnecessary constant string - std::string - appending. Things like the newline adder add a lot of extra compile time while not actually giving less typing than people inserting '\n' by hand (On top of the runtime behavior doing less unnecessary computation)... (3) I've worked with variadic templates on gcc 4.6 - 4.9, and each until about 4.7 has slight changes in behavior / bugfixes. We really need a bot moving into C++11 land to guarantee that all the compilers we say we support work. (1) Good point, we can make strings::join variadic for C++11 and it can do the job for us in help.hpp, to avoid further proliferation of string joiners. :) (3) Ok, let's keep the #ifdef for now! - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review53198 --- 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
Re: Review Request 25079: Replaced macro expansion with variadic template
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51938 --- Patch looks great! Reviews applied: [25079] All tests passed. - Mesos ReviewBot 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
Re: Review Request 25079: Replaced macro expansion with variadic template
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51748 --- I'm no expert in variadic templates, but the rest of the code (moving help impls into cpp) looks great. Somebody with more C++11 skillz should take a look. 3rdparty/libprocess/src/help.cpp https://reviews.apache.org/r/25079/#comment90324 Please add Apache license block. - Adam B On Aug. 27, 2014, 12:57 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 27, 2014, 12:57 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
Re: Review Request 25079: Replaced macro expansion with variadic template
--- 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. Changes --- Adding Apache license block 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 (updated) - 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
Re: Review Request 25079: Replaced macro expansion with variadic template
On Aug. 26, 2014, 5:58 p.m., Ben Mahler wrote: It was done using macro expansion because variadic templates require C++11. We're not yet able to assume C++11: https://issues.apache.org/jira/browse/MESOS-750 Niklas Nielsen wrote: But as this reduce compile time for folks with C++11 compilers, can we make this guarded by #ifdef and enable it for good when we make the transition? +1. It seems like we can handle this version by adding it to the c++11 directory and guarding it with #ifdef similar to defer.hpp. - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51595 --- On Aug. 26, 2014, 5:44 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 26, 2014, 5:44 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
Re: Review Request 25079: Replaced macro expansion with variadic template
On Aug. 26, 2014, 7:36 p.m., Mesos ReviewBot wrote: Bad patch! Reviews applied: [25079] Failed command: ./support/mesos-style.py Error: Checking 506 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo 3rdparty/libprocess/include/process/help.hpp:61: public: should not be indented inside class TLineHelper [whitespace/indent] [3] 3rdparty/libprocess/include/process/help.hpp:72: public: should not be indented inside class TLineHelper [whitespace/indent] [3] 3rdparty/libprocess/src/help.cpp:77: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:78: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:124: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:136: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:139: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:154: Tab found; better to use spaces [whitespace/tab] [1] Total errors found: 8 Probably not you guys, but can you address these style bugs too? You can run ./support/mesos-style.py to get the style report. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51633 --- On Aug. 26, 2014, 2:44 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 26, 2014, 2:44 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
Re: Review Request 25079: Replaced macro expansion with variadic template
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 27, 2014, 5:20 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 (updated) - 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
Re: Review Request 25079: Replaced macro expansion with variadic template
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51666 --- Bad patch! Reviews applied: [25079] Failed command: ./support/mesos-style.py Error: Checking 506 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo 3rdparty/libprocess/include/process/help.hpp:61: public: should not be indented inside class TLineHelper [whitespace/indent] [3] 3rdparty/libprocess/include/process/help.hpp:72: public: should not be indented inside class TLineHelper [whitespace/indent] [3] Total errors found: 2 - Mesos ReviewBot On Aug. 27, 2014, 5:20 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 27, 2014, 5:20 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
Re: Review Request 25079: Replaced macro expansion with variadic template
On Aug. 26, 2014, 9:58 p.m., Ben Mahler wrote: It was done using macro expansion because variadic templates require C++11. We're not yet able to assume C++11: https://issues.apache.org/jira/browse/MESOS-750 Niklas Nielsen wrote: But as this reduce compile time for folks with C++11 compilers, can we make this guarded by #ifdef and enable it for good when we make the transition? Michael Park wrote: +1. It seems like we can handle this version by adding it to the c++11 directory and guarding it with #ifdef similar to defer.hpp. Michael Park please make that change. I guess we will just check with: __cplusplus = 201103L - Patrick --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51595 --- On Aug. 27, 2014, 6:10 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 27, 2014, 6:10 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
Re: Review Request 25079: Replaced macro expansion with variadic template
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 27, 2014, 6:10 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 (updated) - 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
Re: Review Request 25079: Replaced macro expansion with variadic template
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51595 --- It was done using macro expansion because variadic templates require C++11. We're not yet able to assume C++11: https://issues.apache.org/jira/browse/MESOS-750 - Ben Mahler On Aug. 26, 2014, 9:44 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 26, 2014, 9:44 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
Re: Review Request 25079: Replaced macro expansion with variadic template
On Aug. 26, 2014, 2:58 p.m., Ben Mahler wrote: It was done using macro expansion because variadic templates require C++11. We're not yet able to assume C++11: https://issues.apache.org/jira/browse/MESOS-750 But as this reduce compile time for folks with C++11 compilers, can we make this guarded by #ifdef and enable it for good when we make the transition? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51595 --- On Aug. 26, 2014, 2:44 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 26, 2014, 2:44 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
Re: Review Request 25079: Replaced macro expansion with variadic template
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/#review51633 --- Bad patch! Reviews applied: [25079] Failed command: ./support/mesos-style.py Error: Checking 506 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo 3rdparty/libprocess/include/process/help.hpp:61: public: should not be indented inside class TLineHelper [whitespace/indent] [3] 3rdparty/libprocess/include/process/help.hpp:72: public: should not be indented inside class TLineHelper [whitespace/indent] [3] 3rdparty/libprocess/src/help.cpp:77: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:78: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:124: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:136: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:139: Tab found; better to use spaces [whitespace/tab] [1] 3rdparty/libprocess/src/help.cpp:154: Tab found; better to use spaces [whitespace/tab] [1] Total errors found: 8 - Mesos ReviewBot On Aug. 26, 2014, 9:44 p.m., Patrick Reilly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25079/ --- (Updated Aug. 26, 2014, 9:44 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
