Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11843 )
Change subject: [util] Add ParseStringsWithScheme in net_util ...................................................................... Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/11843/1/src/kudu/util/net/net_util.h File src/kudu/util/net/net_util.h: http://gerrit.cloudera.org:8080/#/c/11843/1/src/kudu/util/net/net_util.h@46 PS1, Line 46: // Similar to above but allow the address to be have scheme and path. e.g. Should mention that the scheme and path are ignored. http://gerrit.cloudera.org:8080/#/c/11843/1/src/kudu/util/net/net_util.h@76 PS1, Line 76: // Similar to above but allow the addresses to be have scheme and path. Same. http://gerrit.cloudera.org:8080/#/c/11843/1/src/kudu/util/net/net_util.cc File src/kudu/util/net/net_util.cc: http://gerrit.cloudera.org:8080/#/c/11843/1/src/kudu/util/net/net_util.cc@149 PS1, Line 149: Status HostPort::ParseStringWithScheme(const std::string &str, uint16_t default_port) { No std:: prefix. Also const string& http://gerrit.cloudera.org:8080/#/c/11843/1/src/kudu/util/net/net_util.cc@150 PS1, Line 150: string uri(str); Nit: maybe name this "str_copy" so it's more clear that it's a copy? http://gerrit.cloudera.org:8080/#/c/11843/1/src/kudu/util/net/net_util.cc@153 PS1, Line 153: Maybe you can simplify some of this with gutil functions like StripPrefixString, StripSuffixString, etc? http://gerrit.cloudera.org:8080/#/c/11843/1/src/kudu/util/net/net_util.cc@217 PS1, Line 217: vector<string> addr_strings = strings::Split(comma_sep_addrs, ",", strings::SkipEmpty()); Once you've got this, you can reserve the appropriate capacity in res. http://gerrit.cloudera.org:8080/#/c/11843/1/src/kudu/util/net/net_util.cc@221 PS1, Line 221: res->push_back(host_port); Nit: emplace_back for new code. -- To view, visit http://gerrit.cloudera.org:8080/11843 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2447ab80e0b0737c4eb2ba8216769a52b5c07ce0 Gerrit-Change-Number: 11843 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 01 Nov 2018 04:14:17 +0000 Gerrit-HasComments: Yes
