-
Notifications
You must be signed in to change notification settings - Fork 83
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
Fix the CMake code of the project #85
base: main-dev
Are you sure you want to change the base?
Conversation
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.
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
Certainly. The readme documents both
No problem, help is readily available. Feel free to tag me for anything CMake related or join the |
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 |
430f05e
to
f7e4c05
Compare
19e6998
to
4e33434
Compare
e0a9e4e
to
c8c6c7c
Compare
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
.