Remove global includes and use target include for Apps#6009
Remove global includes and use target include for Apps#6009larshg merged 2 commits intoPointCloudLibrary:masterfrom
Conversation
larshg
changed the title
larshg
changed the title
larshg
force-pushed
the
updateincludesapps
branch
from
784a150 to
56499d3
Compare
|
Could you rebase this on master, please? |
larshg
force-pushed
the
updateincludesapps
branch
from
56499d3 to
7d8a618
Compare
|
|
||
| PCL_ADD_EXECUTABLE(pcl_nn_classification_example COMPONENT ${SUBSYS_NAME} SOURCES src/nn_classification_example.cpp) | ||
| target_link_libraries(pcl_nn_classification_example pcl_common pcl_io pcl_features pcl_kdtree) | ||
| target_link_libraries(pcl_nn_classification_example pcl_apps pcl_common pcl_io pcl_features pcl_kdtree) |
There was a problem hiding this comment.
I don't understand why this has to link to pcl_apps when it didn't have to before? (same for some more executables below)
There was a problem hiding this comment.
Because it includes pcl/apps/vfh_nn_classifier.h - which resides in apps "library".
There was a problem hiding this comment.
Line 21 the old include_directories made this available to all the executables afterwards.
There was a problem hiding this comment.
But vfh_nn_classifier.h seems to be a standalone header with every function fully defined, so no corresponding cpp file that would make linking to pcl_apps necessary. Wouldn't it be better to add target_include_directories(pcl_nn_classification_example ...) instead of unnecessarily adding pcl_apps as a dependency?
There was a problem hiding this comment.
Yeah, that should do it aswell of course. I'll change asap.
2 participants
follows after #6008