-
Notifications
You must be signed in to change notification settings - Fork 115
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
relative includes #30
Comments
Yeah, that makes sense. I think we should be consistent. |
The more time passes, the more I think we should've gone the opposite way on this issue, as @vaijns originally suggested. The rationale is we shouldn't expect users to have to set any extra include directories when using our header-only library as a drop-in. The headers should include each other in a relative manner so no matter where our library is, no matter what the user's build system looks like, if he manages to include one of our files, then it can find others because they are all packaged together anyway. I've been looking around some header-only libraries and this seems to be done quite often, Hanickadot's CTRE is a good example of this practice. I hereby request this issue be reopen, that all includes be changed to relative paths, and that we add somewhere in the README that developers should strictly adhere to this for the stability of the library. I'm currently looking for a This will become even more of a problem the moment we start adding more dependencies. We should solve this now. In the meantime, I'll get a PR going. |
Change project includes to relative (related to #30)
Most includes seem to use an absolute path (starting with "rfl/...") with some exceptions I found:
which have some relative paths defined as well (might wanna unify that).
However I think this makes it a bit harder to include the library just for some quick testing as you have to specify an include directory (which you'd probably do for production anyways though ofc).
For me I like to try out new (header-only) libraries by just
git init
andgit submodule add https://github.com/getml/reflect-cpp libs/reflect-cpp
and using it in a simple main.cpp with#include "libs/reflect-cpp/include/rfl.hpp"
, running clang/gcc for compilation directly.This doesn't work with these absolute paths though as:
rfl.hpp
includesrfl/AllOf.hpp
(fine so far) which then includesrfl/Result.hpp
which is at the same level, as we are already in therfl
directory. So the include trying to go intorfl
a level deeper doesn't work unless I addlibs/reflect-cpp/include/
as an include directory (compiling withg++ -std=c++20 -I ./libs/reflect-cpp/include/ main.cpp
), so it can always go "back" into the include directories rootrfl
-directory while with relative paths it should just work right out of the box without that.also the described way to
doesn't quite work due to this (unless the 'OR' is meant to be an 'and' instead).
Not a big deal of course and relative paths have their cons while absolute paths have their pros, so you might wanna consider changing it but it's fine if you don't.
btw., that's a cool and useful library, definitely gonna consider using it for future projects. :)
The text was updated successfully, but these errors were encountered: