----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37240/#review94827 -----------------------------------------------------------
Ship it! 3rdparty/libprocess/include/process/http.hpp (line 112) <https://reviews.apache.org/r/37240/#comment149470> Hm.. is the http path similar enough to POSIX paths? 3rdparty/libprocess/src/process.cpp (lines 3029 - 3030) <https://reviews.apache.org/r/37240/#comment149485> Seems a little odd to have this comment up here and then a similar comment above the while loop, can you combine them into one? Either a bigger one here that describes how we look, or just keep the one above the loop. 3rdparty/libprocess/src/process.cpp (line 3039) <https://reviews.apache.org/r/37240/#comment149476> The following seems a bit less hacky than hardcoding "" and "." to know when we're done? ``` while (Path(name).dirname() != name) { } ``` This also avoids relying on "/" being trimmed (since you're not checking for name == "/" we must have that trim code in place). 3rdparty/libprocess/src/process.cpp (line 3040) <https://reviews.apache.org/r/37240/#comment149474> Maybe pull this up outside of the loop? Not sure what value there is in logging each step, also VLOG(1) seems too verbose for this kind of thing? 3rdparty/libprocess/src/process.cpp (lines 3065 - 3066) <https://reviews.apache.org/r/37240/#comment149481> Mind adding a newline for readability? 3rdparty/libprocess/src/tests/http_tests.cpp (line 451) <https://reviews.apache.org/r/37240/#comment149469> Why the semi colon? 3rdparty/libprocess/src/tests/http_tests.cpp (line 455) <https://reviews.apache.org/r/37240/#comment149468> Maybe this is a bit more explicit about which statuses you're expecting: ``` ASSERT_EQ(http::OK().status, ...); ``` Ditto for 202? - Ben Mahler On Aug. 7, 2015, 11:39 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37240/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2015, 11:39 p.m.) > > > Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Ben Mahler. > > > Bugs: MESOS-3237 > https://issues.apache.org/jira/browse/MESOS-3237 > > > Repository: mesos > > > Description > ------- > > Needed this for scheduler HTTP API to handle "/api/v1/scheduler" > > > Diffs > ----- > > 3rdparty/libprocess/include/process/http.hpp > f24ca24f4b2926d6d9651b90bdf4dd8156f71c9f > 3rdparty/libprocess/src/process.cpp > 2ce547bd3dc13841cc6ea2537df1398acdb1edef > 3rdparty/libprocess/src/tests/http_tests.cpp > ecbcbd552ac834659860627c82628ed38e6139b3 > > Diff: https://reviews.apache.org/r/37240/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
