-
Notifications
You must be signed in to change notification settings - Fork 4
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
Compile based on present architecture #127
base: main
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.
yes, lgtm. I won't have to add it manually
Do we need similar options in the |
hum, actually none of the flags in that line are picked up by cmake it seems. Let me double check |
(but changing them directly in cmake also propagates it for the tests, so that part is fine) |
Because these variables are internal ones defined already by cmake they had to be overwritten and a test had to be added so that users could still use |
I've tried locally and all good for me, but the GitHub action failed here in the PR, do you have any idea why? |
do we know which version of cmake the CI uses? it doesn't seem to detect openmp related things properly. This isn't something that was introduced with this PR but flags set here never got actually used before so we see it only now |
No description provided.