> On July 17, 2015, 7:13 a.m., haosdent huang wrote: > > src/tests/utils.hpp, line 72 > > <https://reviews.apache.org/r/36501/diff/3/?file=1013117#file1013117line72> > > > > How about check path length and use `path[0]` here? Instead of > > `*(path.begin())`
I think we can use startsWith to check it. > On July 17, 2015, 7:13 a.m., haosdent huang wrote: > > src/tests/utils.hpp, line 69 > > <https://reviews.apache.org/r/36501/diff/3/?file=1013117#file1013117line69> > > > > Indent also not correct here. And if > > `net::getHostname(process.self().address.ip)` return error, does it make > > incorrect url? Yes, agree with that. If we want to build a new function, Try<std::string> should be the type of return value. > On July 17, 2015, 7:13 a.m., haosdent huang wrote: > > src/tests/utils.hpp, line 55 > > <https://reviews.apache.org/r/36501/diff/3/?file=1013117#file1013117line55> > > > > 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. Agree. > On July 17, 2015, 7:13 a.m., haosdent huang wrote: > > src/tests/utils.hpp, line 60 > > <https://reviews.apache.org/r/36501/diff/3/?file=1013117#file1013117line60> > > > > I still suggest use <string> method and remove <cstring> Yes; if we can remove it because of endsWith util fuction. > On July 17, 2015, 7:13 a.m., haosdent huang wrote: > > src/tests/utils.hpp, line 64 > > <https://reviews.apache.org/r/36501/diff/3/?file=1013117#file1013117line64> > > > > 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 || > > ``` Thanks; just found endsWith in our source code. - Klaus ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36501/#review92033 ----------------------------------------------------------- On July 17, 2015, 2:06 p.m., Klaus Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36501/ > ----------------------------------------------------------- > > (Updated July 17, 2015, 2:06 p.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 > > 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 > >
