ros-industrial / stomp_ros

ROS packages for the STOMP planner (split out of industrial_moveit)
Apache License 2.0
37 stars 27 forks source link

Cleanup and update of build scripts and package manifests #2

Closed gavanderhoorn closed 5 years ago

gavanderhoorn commented 5 years ago

As per title.

No functional changes to any part of the code.

Packages manifests were missing quite some dependencies, but due to transitive dependencies, things still compiled. That is brittle though, so I made many of them explicit in the packages where they were first-level includes.

gavanderhoorn commented 5 years ago

This is still not perfect, but catkin_lint -W2 is a lot happier now:

stomp_core: notice: package description starts with boilerplate 'The stomp_core'
stomp_moveit: notice: package description starts with boilerplate 'The stomp_moveit'
stomp_plugins: notice: package description starts with boilerplate 'The stomp_plugins'
catkin_lint: checked 3 packages and found 3 problems

@jrgnicho: if you'd take care of updating the package descriptions .. ;)

jrgnicho commented 5 years ago

So all those catkin_lint warnings just mean that the description has to be something other that the package name?

gavanderhoorn commented 5 years ago

So all those catkin_lint warnings just mean that the description has to be something other that the package name?

yes, sort of.

catkin_lint doesn't like it very much if the description merely says: "This is the X package".

And it makes sense to flag that, as we already know it's the X package, as that is the name of the package.

gavanderhoorn commented 5 years ago

Thanks for the review @jrgnicho.

Note that there may still be issues with the build scripts, but we quickly find out when we release them.

gavanderhoorn commented 5 years ago

Just noticed that previous builds on Melodic actually failed (this particular one due to a missing tf dependency).

This PR probably fixes that as well.

jrgnicho commented 5 years ago

OK, I'll a more meaningful description there. Thanks