----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36501/#review92033 -----------------------------------------------------------
src/tests/utils.hpp (line 30) <https://reviews.apache.org/r/36501/#comment145909> Need from the style guide: ``` #include <process/process.hpp> #include <stout/json.hpp> #include <stout/option.hpp> ``` src/tests/utils.hpp (line 55) <https://reviews.apache.org/r/36501/#comment145910> Because we use "We use lowerCamelCase for function names (Google uses mixed case for regular functions; and their accessors and mutators match the name of the variable)." So maybe need to change it like this: ``` template <class T> std::string endpointUrl( process::Process<T>& process, const std::string& path, ....) ``` The indent also break the style guild rule here. src/tests/utils.hpp (line 60) <https://reviews.apache.org/r/36501/#comment145911> I still suggest use <string> method and remove <cstring> src/tests/utils.hpp (line 64) <https://reviews.apache.org/r/36501/#comment145912> We have endsWith method in strings.hpp. Maybe use could use it. Also the style looks not correct here. Maybe need chang it like this: ``` if (protocal.length() <= len || ``` src/tests/utils.hpp (line 69) <https://reviews.apache.org/r/36501/#comment145913> Indent also not correct here. And if `net::getHostname(process.self().address.ip)` return error, does it make incorrect url? src/tests/utils.hpp (line 72) <https://reviews.apache.org/r/36501/#comment145914> How about check path length and use `path[0]` here? Instead of `*(path.begin())` - haosdent huang On July 16, 2015, 5:47 a.m., Klaus Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36501/ > ----------------------------------------------------------- > > (Updated July 16, 2015, 5:47 a.m.) > > > Review request for mesos. > > > Bugs: MESOS-3023 > https://issues.apache.org/jira/browse/MESOS-3023 > > > Repository: mesos > > > Description > ------- > > Fix for MESOS-3023 (Factoring out the pattern for URL generation) > > > Diffs > ----- > > src/tests/fetcher_tests.cpp ae10c42 > src/tests/utils.hpp f2eed2e > > Diff: https://reviews.apache.org/r/36501/diff/ > > > Testing > ------- > > 1. Build successfully in Linux > 2. Test passed by src/mesos-tests --gtest_filter=FetcherTest.OSNetUriTest > > > Thanks, > > Klaus Ma > >
