-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Header files layout #15
Conversation
Tests use the library like an external one, so they shoud use <> instead of "" to include header files.
Only use #include "" toward subdirectories.
This shows that tests rely on "impl" details.
I'm okay with putting things into a |
I don't know why |
I will update it after #14 is merged. It will be great to have the install tests! |
I think I would also prefer:
The main offender right now are
Maybe the things needed by namespace KokkosComm {
using Impl::Req;
using Impl::Invokable;
...
} It's a bit redundant but it means
|
Make sense, will do. |
I think this will be somewhat (or entirely) resolved by #109 |
Done in #109. |
Try to follow https://blogs.stonesteps.ca/1/p/64 for including header files.
Also headers now have only one way dependencies between
/
and/impl
.Some clean-up for CMake, like explicit list of files (*.h is notoriously bad for testing, for example, if we forget to add a new file, CMake configuration will still pass).
One question: since it is a new library, can we try to put header files in a
KokkosComm
directory?User code will become:
I can add this to this PR or do a separate one.