----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16839/#review31846 -----------------------------------------------------------
Ship it! This is great Charlie! Mostly nits and nats on style. I'll get this committed after you update. 3rdparty/libprocess/src/process.cpp <https://reviews.apache.org/r/16839/#comment60597> s/if(/if (/ Also, can we put a newline before the 'if' please? 3rdparty/libprocess/src/process.cpp <https://reviews.apache.org/r/16839/#comment60598> I'd love to see a TODO which let's us specify the content type as a 6th parameter. In the mean time, can you leave a comment as to why you picked application/x-ww-form-urlencoded as the default? 3rdparty/libprocess/src/process.cpp <https://reviews.apache.org/r/16839/#comment60612> Yes, let's have the interface take an 'const Option<String>& query'. You can use Some("constant string") when the compiler gives you crap for trying to go from a const char * to an Option<string>. 3rdparty/libprocess/src/process.cpp <https://reviews.apache.org/r/16839/#comment60599> Agreed with Jie, let's take an optional 'query'. Also, for future use, you can just use 'None()' instead of 'Option<string>::none()'. 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60600> Please remove the whitespace around the parameter. 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60611> EXPECT_TRUE? 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60601> Please remove the whitespace around the parameter. 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60610> Kill spaces around '_' please. 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60609> Kill spaces around '_' please. 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60602> Please wrap after the '='. 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60603> Two newlines between top-level declarations please. 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60607> Kill spaces around parameter please. 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60608> EXPECT_TRUE? 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60604> Another newline please. 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60606> Kill spaces around '_' please. 3rdparty/libprocess/src/tests/http_tests.cpp <https://reviews.apache.org/r/16839/#comment60605> Wrap after '=' please. - Benjamin Hindman On Jan. 15, 2014, 2:05 a.m., Charlie Carson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16839/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2014, 2:05 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, Jeff Currier, and Jie > Yu. > > > Bugs: https://issues.apache.org/jira/browse/MESOS-902 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-902 > > > Repository: mesos-git > > > Description > ------- > > Add post function to http.hpp of libprocess. > > This adds the post equivalent of the get function to http.hpp. > > The existing get method is refactored into internal::httpRequest > with arguments for method, query, and body. > > The get and post functions can then be implemented as simple > wrapper of internal::httpRequest. > > There are also new unit tests to verify the existing behavior of > get and the new post behavior. > > See: https://issues.apache.org/jira/browse/MESOS-902 > > Review: https://reviews.apache.org/r/16839 > > > Diffs > ----- > > 3rdparty/libprocess/include/process/http.hpp > 5bdd520c9e0bcc0a2d3a4554cc4ced99dcf78b51 > 3rdparty/libprocess/src/process.cpp > 67f7f9b3b05c8bc9b0f6281689223996ddfa68d1 > 3rdparty/libprocess/src/tests/http_tests.cpp > 68d1a1b2ed5645d861b1e613aed9731368ebdc6a > > Diff: https://reviews.apache.org/r/16839/diff/ > > > Testing > ------- > > added new unit tests > make check > > > Thanks, > > Charlie Carson > >
