> On Dec. 1, 2015, 9:08 p.m., Michael Park wrote: > > src/slave/containerizer/fetcher.hpp, line 43 > > <https://reviews.apache.org/r/40631/diff/2/?file=1139547#file1139547line43> > > > > 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`.
Agree :). I'll update them accordingly. - Klaus ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40631/#review108487 ----------------------------------------------------------- On Dec. 1, 2015, 9:13 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, 9:13 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 > ----- > > src/slave/containerizer/fetcher.hpp 78e7d14 > src/tests/mesos.hpp a2a76f5 > > Diff: https://reviews.apache.org/r/40631/diff/ > > > Testing > ------- > > make && make check > > > Thanks, > > Klaus Ma > >
