Open friendlyanon opened 7 months ago
This is amazing, @friendlyanon! Thanks! As you see, I'm not a great CMake user 😅
The main goal of this PR is to point out issues and provide solutions to some of them. I would have opened an issue first, but issues can't do that effectively.
Things like fixing MSVC support are beyond this PR and will require more work on your end. I don't have the knowledge to properly address this for example.
CI workflows are also left untouched, since this PR is certainly not a final solution. You would also need to add presets similar to this: [1] [2]
What would be a good next step to fix the remaining issues you've highlighted?
You would definitely need to find a way to acquire a Windows developer environment to deal with MSVC issues. Documenting how to get MSVC to work in a mostly standards compliant fashion would also be a bonus ([3]). I would also like to see the CMake install interface being the only documented way of consuming the project. Supporting a vendoring scenario should merely be a side effect of following CMake best practices with some extra on top (like the ${warning_guard}
variable).
Does this break some of the documentation I have?
Certainly. The readme documents both #include <stringzilla.hpp>
and #include <stringzilla/stringzilla.hpp>
. The install rules solidify #include <stringzilla/stringzilla.hpp>
.
As you see, I'm not a great CMake user
No problem, help is readily available. Feel free to tag me for anything CMake related or join the #cmake
channel of the C++ Slack.
Additionally, I would also look into splitting the C "package" into something separate. Seeing how this project is already packaged by Conan, but only the header-only C++ part of it: https://conan.io/center/recipes/stringzilla
Some things this fixes:
add_subdirectory
).stringzillite
seems very suspect still. The developer targets are still very much broken.CMAKE_*
variables, which is verboten.message
calls.BUILD_TESTING
andCMAKE_SKIP_INSTALL_RULES
.