-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Install space compatibility #385
base: master
Are you sure you want to change the base?
Conversation
... since I have no idea how to fix them.
This also fixes a compiler warning about misleading indentation.
Can one of the admins verify this patch? |
Hi, there are quite a few merge conflicts since the changes to make Kalibr python3 compatible. Do you have time to resolve these? I think many would appreciate install space compatibility so they can build in a ROS 2 workspace. |
Hey, I unfortunately don't have time to work on this currently. Maybe in a few weeks. If someone wants to take this up in the meantime, feel free :) |
Great. No rush, I will leave this open until someone picks it up down the line, thanks! |
It seems that kalibr has been used so far only in the catkin devel space, since many of the sub-packages contain no
install()
commands or even wrong / obviously untested ones. However, install spaces are used in many contexts (e.g. binary deployment) and next-gen build systems like colcon (for ROS 2) do not support devel spaces anymore, so it is quite desirable to support installation.This PR adds
install()
commands when required. It also fixes some unit tests that weren't compiling for me usingcolcon
.