> On Jan. 29, 2015, 3:40 p.m., Alexander Rukletsov wrote: > > src/etcd/url.hpp, line 23 > > <https://reviews.apache.org/r/30394/diff/1/?file=839637#file839637line23> > > > > `#include <iostream>` > > or, what I would prefer > > `#include <iosfwd>` > > and move `operator<<` definitions into `url.cpp`
Strings already includes <iostream> so it's not necessary here. From scanning the codebase the standard pattern currently is to define these inline in the headers from a scan of the codebase. > On Jan. 29, 2015, 3:40 p.m., Alexander Rukletsov wrote: > > src/etcd/url.cpp, line 18 > > <https://reviews.apache.org/r/30394/diff/1/?file=839638#file839638line18> > > > > Shouldn't we add `<cassert>`? I'm following the pattern of zookeeper here which doesn't do it. The code already errors if an empty string is passed (with a runtime error, which it is rather than a programmer logic error which is an assert), and the codebase doesn't assert references are non-null (even though they can be), so I don't see where I should add asserts. - Cody ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30394/#review70206 ----------------------------------------------------------- On Jan. 29, 2015, 3:58 a.m., Cody Maloney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30394/ > ----------------------------------------------------------- > > (Updated Jan. 29, 2015, 3:58 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/30394/diff/ > > > Testing > ------- > > > Thanks, > > Cody Maloney > >
