----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11710/#review21689 -----------------------------------------------------------
I'm confused as to why you decided to expose the Help process in include? Was it out of necessity? It seems like we would want to hide the Help Process and only use it internally. 3rdparty/libprocess/include/process/help.hpp <https://reviews.apache.org/r/11710/#comment44824> If you're only needing 1-2 levels of headers, I find the Setext header format more readable in the text form, but I believe you only get h1 and h2. See: http://daringfireball.net/projects/markdown/syntax#header 3rdparty/libprocess/include/process/help.hpp <https://reviews.apache.org/r/11710/#comment44828> I see strings::join doesn't take 6 args ;) should we add more overloads? should we consider a "joiner" style utility? strings::join("\n") << tldr << usage << description; // would a string cast operator on the stream type we create be sufficient in most cases? We could also use the template generating magic as used in this file. Does C++11 make it easier? 3rdparty/libprocess/include/process/help.hpp <https://reviews.apache.org/r/11710/#comment44830> seems like keeping the newlines on the same line makes more sense and would read better: tldr + "\n" + "### USAGE\n" + 3rdparty/libprocess/include/process/help.hpp <https://reviews.apache.org/r/11710/#comment44836> s/2/3/ 3rdparty/libprocess/include/process/help.hpp <https://reviews.apache.org/r/11710/#comment44840> const&? 3rdparty/libprocess/include/process/help.hpp <https://reviews.apache.org/r/11710/#comment44841> Not handling >2 as a bad request? 3rdparty/libprocess/include/process/help.hpp <https://reviews.apache.org/r/11710/#comment44848> Request.accepts("text/html") will tell you that, if you want it. 3rdparty/libprocess/include/process/help.hpp <https://reviews.apache.org/r/11710/#comment44845> Do you want this html to contain newlines for readability in the page source? 3rdparty/libprocess/include/process/help.hpp <https://reviews.apache.org/r/11710/#comment44844> map because you want ordered, right? 3rdparty/libprocess/include/process/logging.hpp <https://reviews.apache.org/r/11710/#comment44856> It's too bad you can't initialize it in this file.. 3rdparty/libprocess/src/process.cpp <https://reviews.apache.org/r/11710/#comment44855> Ah I see this is one character shy of fitting on one line! Then I'm guessing you continued the trend, but I think the others read easier without the unnecessary newlines. 3rdparty/libprocess/src/process.cpp <https://reviews.apache.org/r/11710/#comment44854> Why the newlines? 3rdparty/libprocess/src/process.cpp <https://reviews.apache.org/r/11710/#comment44853> Why the newlines? 3rdparty/libprocess/src/statistics.cpp <https://reviews.apache.org/r/11710/#comment44850> why the newlines? 3rdparty/libprocess/src/statistics.cpp <https://reviews.apache.org/r/11710/#comment44851> why the newlines? - Ben Mahler On June 7, 2013, 5:51 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11710/ > ----------------------------------------------------------- > > (Updated June 7, 2013, 5:51 p.m.) > > > Review request for mesos, Vinod Kone and Ben Mahler. > > > Description > ------- > > Help! I need somebody. HELP! Not just anybody. > > > Diffs > ----- > > 3rdparty/libprocess/Makefile.am 9facdd99e502e6d0470ef3bb5715c88a4562dd20 > 3rdparty/libprocess/include/process/help.hpp PRE-CREATION > 3rdparty/libprocess/include/process/logging.hpp > cba2fd4b52f1aeaa2cd9538ded431c7850c2fe2a > 3rdparty/libprocess/include/process/process.hpp > e70b4f7c521658443c84578876fb4d9a30688b03 > 3rdparty/libprocess/include/process/profiler.hpp > 64cf6224a73a5f4ec26050e9e62c7bc4a0cea1f7 > 3rdparty/libprocess/src/process.cpp > 3ffe0b52b9981d1666d53a0f2d62f9aba6f0a969 > 3rdparty/libprocess/src/statistics.cpp > d8f5ad10434f0cf4110e399db7c07f14810fae60 > 3rdparty/libprocess/src/tests/http_tests.cpp > f6772674e2fd795b92c7ea787fa41e12f8af6fc2 > > Diff: https://reviews.apache.org/r/11710/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
