----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40631/#review107741 -----------------------------------------------------------
src/tests/mesos.hpp (line 87) <https://reviews.apache.org/r/40631/#comment167027> This seems like a weird addition for this patch: while the existing `using` decls above could be justified (it's hard to imagine testing test infrastructure w/o having to perform some environment cleanup), unconditionally pulling in `mesos::fetcher::FetcherInfo` here seems way too general; also, it probably would make any attempt of e.g., mocking `FetcherInfo` anywhere much harder. Either pull this into the internal namespace, or just do the extra typing here and pull it in in the corresponding `mesos.cpp`. - Benjamin Bannier On Nov. 24, 2015, 9:19 a.m., Klaus Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40631/ > ----------------------------------------------------------- > > (Updated Nov. 24, 2015, 9:19 a.m.) > > > Review request for mesos. > > > Bugs: MESOS-3963 > https://issues.apache.org/jira/browse/MESOS-3963 > > > Repository: mesos > > > Description > ------- > > According to the google code style, the using should be used in internal > namespace in header files. Grep the header files, only fetcher.hpp deserved a > path. > > > > You may use a using-declaration anywhere in a .cc file (including in the > > global namespace), and in functions, methods, classes, or within internal > > namespaces in .h files. > > >Do not use using-declarations in .h files except in explicitly marked > >internal-only namespaces, because anything imported into a namespace in a .h > >file becomes part of the public API exported by that file. > > ``` > // OK in .cc files. > // Must be in a function, method, internal namespace, or > // class in .h files. > using ::foo::bar; > ``` > > > Diffs > ----- > > src/slave/containerizer/fetcher.hpp > 78e7d145328c9f7aa9646fa7d6d92f834010053f > src/tests/mesos.hpp a2a76f55f9a4091cccfccd9672c2d2897b34ddcd > > Diff: https://reviews.apache.org/r/40631/diff/ > > > Testing > ------- > > make && make check > > > Thanks, > > Klaus Ma > >
