Open cedricchevalier19 opened 5 months ago
I'm okay with putting things into a KokkosComm
directory - @masterleinad do you know if Trilinos has an opinion on the structure of the headers?
I'm okay with putting things into a
KokkosComm
directory - @masterleinad do you know if Trilinos has an opinion on the structure of the headers?
I don't know why Trilinos
would care. It doesn't matter for TriBITS
but I don't know if there are any other Trilinos
policies regarding subdirectories for header files vs prefixing (or both).
I will update it after #14 is merged. It will be great to have the install tests!
I think I would also prefer:
impl/*
impl/*
CANNOT include things from public headerThe main offender right now are
KokkosComm::Req
(used in the async MPI Impl::
s and the packers)impl/KokkosComm_concepts.hpp
(defines some things not in the Impl::
namespace, oops, needs to be fixed.)Maybe the things needed by Impl::
should actually be moved into Impl::
, and then we can export them up into the public namespace:
namespace KokkosComm {
using Impl::Req;
using Impl::Invokable;
...
}
It's a bit redundant but it means
impl/*
files that need the Req
and concepts can use them without including a non-impl header, and then we can also have those APIs available outside of Impl::
, which is what we're intending for the users.Make sense, will do.
I think this will be somewhat (or entirely) resolved by https://github.com/kokkos/kokkos-comm/pull/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.