search

ros-perception / laser_geometry

Provides the LaserProjection class for turning laser scan data into point clouds.
BSD 3-Clause "New" or "Revised" License
157 stars 114 forks source link

use target_include_directories #54

Closed Karsten1987 closed 4 years ago

Karsten1987 commented 4 years ago

fixes https://github.com/ros-perception/laser_geometry/issues/53

please also consider this for a backport to eloquent.

Signed-off-by: Karsten Knese karsten@openrobotics.org

Karsten1987 commented 4 years ago

for what it's worth: I am not part of the org unit and can't assign myself to this issue/PR

Karsten1987 commented 4 years ago
Karsten1987 commented 4 years ago

@wjwwood just a friendly ping as you're listed as the maintainer. Not sure if you're watching this repo.

wjwwood commented 4 years ago

@jonbinney is helping, but maybe never added himself to the package.xml.

jonbinney commented 4 years ago

I thought I was safe if I wasn't in the package xml! :-P

I'll review this today.

wjwwood commented 4 years ago

Not so long as I'm around :p

But seriously, @jonbinney feel free ask me for help if you don't have time.

Karsten1987 commented 4 years ago

@jonbinney I hope the change is trivial enough that it doesn't take much to review it.

It's essentially really just avoiding a global include_directories CMake instructions which has implications to other projects when built in non-isolation. The change only attaches the includes to the actual CMake target.

jonbinney commented 4 years ago

Yeah, i mainly wanted to build it myself as a sanity check. It does build for me, and tests pass (yay!). I've left one comment with a question, once that is answered I'll merge.