-
Notifications
You must be signed in to change notification settings - Fork 14
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
Improvement suggestions #1
Comments
I have solved some of these issues. I will comment the ones that are remaining.
Not sure how if the compilation procedure usting catkin_tools. Do I need to clone all the packages as explained here: https://catkin-tools.readthedocs.io/en/latest/verbs/catkin_build.html?
I opted to keep
This is indeed important, I will keep this task for future |
To use catkin tools, you need the the workspace to be set up for catkin tools. There is a migration guide. The |
Thank you for the explanation. Now it is missing only the examples. I need to think first how to do it. |
README.md
recommend using catkin_tools (catkin build
command) instead of the deprecated catkin_make.package.xml
unspecified version. Verion 2 or 3 is now supported on all platforms.package.xml
doesn't list pinocchio as a dependency. Check if the ROS binaries are working with the plugin.#pragma once
instead of the#ifdef
guard. All modern compilers support it now (GCC3.4+).catkin_package()
can be left empty in the cmakelist. The headers also don't need to be installed. This will prevent users accidentally linking against the plugin, which may cause problems./** ... **/
type doxygen comments annotate the following line. The header files use these to describe a group of variables but only the first variable will contain the description. Either use the member groups feature or just switch them to regular comments.The code is well structured and documented. Well done.
The text was updated successfully, but these errors were encountered: