-
Couldn't load subscription status.
- Fork 1.5k
fixed #14002 - added missing Boost include for testrunner
#7660
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
Conversation
testrunner
|
See https://github.com/danmar/cppcheck/actions/runs/16191580303 for failed builds with the newly added job and no fix applied. |
|
#7658 will fix the issue with boost. |
| target_externals_include_directories(testrunner PRIVATE ${PROJECT_SOURCE_DIR}/externals/simplecpp/) | ||
| if (Boost_FOUND) | ||
| target_include_directories(testrunner SYSTEM PRIVATE ${Boost_INCLUDE_DIRS}) | ||
| endif() |
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.
Having to updated all the consumers when dependencies change is very fragile and error-prone. #7658 fixes that issue.
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, but that should not be merged so close to the release. And fixing such a minor issue within implicitly in a major rework is not great.
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.
Its always a small fix while technical debt piles up. Its best to take the opportunity to fix the issue once and for all.
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.
We can merge this for the short-term.
|
@danmar @orbitcowboy @chrchr-github Can you merge this? |
fixes build with Boost installed in non-default system include dir