----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40631/#review108487 -----------------------------------------------------------
src/slave/containerizer/fetcher.hpp (line 41) <https://reviews.apache.org/r/40631/#comment167944> I agree with the intent to remove the using declaration out of `fetcher.hpp`, but I don't agree with moving it into an internal namespace. Fetcher-related cpp files should pull it in itself; `src/launcher/fetcher.cpp` for example already does so. I strongly disagree with introducing a using declaration to `src/tests/mesos.hpp`. It doesn't seem to be something that should be common across all tests. It should be local to fetcher-related tests; `src/tests/fetcher_tests.cpp` for example already pulls it in itself. My suggestion would be to use using declarations in cpp files, and use qualified names in hpp files. We end up with the following changes: (1) Add `using mesos::fetcher::FetcherInfo;` in `src/slave/containerizer/fetcher.cpp` and `src/tests/fetcher_cache_tests.cpp`. (2) `s/FetcherInfo/fetcher::FetcherInfo/` for 1 instance in `src/slave/containerizer/fetcher.hpp` and 3 instances in `src/tests/mesos.cpp`. - Michael Park On Dec. 1, 2015, 12:51 p.m., Klaus Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40631/ > ----------------------------------------------------------- > > (Updated Dec. 1, 2015, 12:51 p.m.) > > > Review request for mesos, Benjamin Bannier, Joseph Wu, and Michael Park. > > > 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 > ----- > > include/mesos/mesos.proto 27971fe > include/mesos/v1/mesos.proto 9acefd5 > src/common/resources.cpp 98804a4 > src/tests/resources_tests.cpp dbd39cd > src/v1/resources.cpp 8488c31 > > Diff: https://reviews.apache.org/r/40631/diff/ > > > Testing > ------- > > make && make check > > > Thanks, > > Klaus Ma > >
