----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30588/#review71394 -----------------------------------------------------------
src/Makefile.am <https://reviews.apache.org/r/30588/#comment117131> Please align the '\' (and align the others below you if they're not, thanks!). src/etcd/etcd.cpp <https://reviews.apache.org/r/30588/#comment117132> s/ttl/TTL/ Captilize acroynms please! src/etcd/etcd.cpp <https://reviews.apache.org/r/30588/#comment117133> s/.// src/etcd/etcd.cpp <https://reviews.apache.org/r/30588/#comment117135> s/auto/Address/ s/host/address/ src/etcd/etcd.cpp <https://reviews.apache.org/r/30588/#comment117136> Newline here please! src/etcd/etcd.cpp <https://reviews.apache.org/r/30588/#comment117137> Newline please! src/etcd/etcd.cpp <https://reviews.apache.org/r/30588/#comment117138> Dead code! src/etcd/etcd.cpp <https://reviews.apache.org/r/30588/#comment117141> How about a newline before you start the next "stanza" of text to show the reader that this is something else? src/etcd/etcd.cpp <https://reviews.apache.org/r/30588/#comment117139> No snake_case please! src/etcd/etcd.cpp <https://reviews.apache.org/r/30588/#comment117140> No snake_case please! src/etcd/url.hpp <https://reviews.apache.org/r/30588/#comment117142> I really don't see why we can't use an Address here? src/etcd/url.hpp <https://reviews.apache.org/r/30588/#comment117143> Is it a server or a host? ;-) But let's just use Address! src/etcd/url.hpp <https://reviews.apache.org/r/30588/#comment117144> '{' on newline! (For the struct above too, but let's just use Address!). src/etcd/url.cpp <https://reviews.apache.org/r/30588/#comment117145> s/7/strlen(etcd::URL::scheme())/ src/etcd/url.cpp <https://reviews.apache.org/r/30588/#comment117146> Is it hosts or servers!? src/etcd/url.cpp <https://reviews.apache.org/r/30588/#comment117147> Try<vector<Address>> resolve = network::resolve(server); if (resolve.isError()) { ... } if (resolve.get().empty()) { ... } // Just take the first address and give it a default port if necessary. Address address = resolve.get().front(); address.port = address.port == 0 ? 4001; addresses.push_back(address); src/etcd/url.cpp <https://reviews.apache.org/r/30588/#comment117149> if (!strings::startsWith(pathStart, "/v2/keys/")) { src/etcd/url.cpp <https://reviews.apache.org/r/30588/#comment117148> s/-1/ - 1/ - Benjamin Hindman On Feb. 4, 2015, 12:10 a.m., Cody Maloney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30588/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2015, 12:10 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-1806 > https://issues.apache.org/jira/browse/MESOS-1806 > > > Repository: mesos > > > Description > ------- > > etcd api wrapper > > > Diffs > ----- > > src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e > src/etcd/etcd.hpp PRE-CREATION > src/etcd/etcd.cpp PRE-CREATION > src/etcd/url.hpp PRE-CREATION > src/etcd/url.cpp PRE-CREATION > src/master/constants.hpp c386eab3973363e64c113f7e84452456fdd3393c > src/master/constants.cpp 9ee17e9ad884da98cabfdfe316cb4a831de193b3 > > Diff: https://reviews.apache.org/r/30588/diff/ > > > Testing > ------- > > > Thanks, > > Cody Maloney > >
