-
Notifications
You must be signed in to change notification settings - Fork 1k
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
cmake: Rework flags summary #1558
base: master
Are you sure you want to change the base?
Conversation
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.
Concept ACK
The note is a bit verbose, but I don't have a convincing suggestion to shorten it.
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.
Sorry, when I wrote the previous comment, I assumed this PR is in the hebasto/bitcoin repo for cmake staging... 🤦♂️ :
utACK aa4c5f3 this matches the implementation in hebasto/bitcoin#93 and has been revieed there
Looks like this is at least missing link options like |
Also flags like |
Good observations. These should be fixed if the rule is as in this comment:
|
aa4c5f3
to
765681b
Compare
Both @fanquake's comments have been addressed. |
The same is still missing on Linux? |
6fbe2bc
to
cb574bf
Compare
All recent @fanquake's comments have been addressed. |
cb574bf
to
8ff8b25
Compare
Fixed quoting of (potentially empty) string arguments. |
This was requested in hebasto/bitcoin#221. The implementation follows the same approach as in hebasto/bitcoin#93.
Here are a few excerpts from the summaries: