-
Notifications
You must be signed in to change notification settings - Fork 38
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
make install with packaging #40
Conversation
@@ -1,12 +1,13 @@ | |||
cmake_minimum_required(VERSION 3.4) | |||
cmake_minimum_required(VERSION 3.4...3.28) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the 3.28
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only because I have cmake 3.28.2.
I may try it on 3.31 later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @agladilin, please note that I had to revert this change as it caused clang-diagnostic-error
with gcc. See the failure. I reverted the use of policy_max
from the cmake_minimum_required
and now all checks pass. See the result after the fix.
You can create another PR to add this back and maybe some other changes are required too to avoid the clang-diagnostic-error
.
Thank you for the improvement. I will take a detail look as there are few cmake constructs I am not very familiar with. I will soon do the full review. |
These changes actually generate the installation tree along with cmake files that allow to use the
I don't specify the final location of the installed cppparser in my project. Tested on Ubuntu 22.04 and Windows with Visual Studio. |
No description provided.