[POS-51] Update the FindStarling modules #52
Conversation
jbangelo
changed the title
| mark_target_as_system_includes(sensorfusion) | ||
| mark_target_as_system_includes(pvt_driver) | ||
| mark_target_as_system_includes(pvt-engine) | ||
| mark_target_as_system_includes(pvt-common) | ||
| mark_target_as_system_includes(starling-util) |
There was a problem hiding this comment.
should these check to make sure that the targets exists before marking them? if (TARGET sensorfusion) ... endif(), on the off chance that we might rename/remove some of these targets in the future.
There was a problem hiding this comment.
It looks like mark_target_as_system_includes will silently do nothing if the targets don't exist.
Are you thinking that we should be failing or giving a warning if the targets don't exist?
There was a problem hiding this comment.
I didn't expect that behaviour of the mark_target_as_system_includes function (sorry didn't dig to deep into it). I'd probably go with a warning, mostly because cmake is an external library library.
There was a problem hiding this comment.
my comment is really just a nit. I'll leave matt to approve the request since he knows this portion of the cmake work inside out. I don't see any isssues with this PR, I'm happy to approve.
This updates the
FindStarlingmodules to find the new interface library for the PVT engine.