-
Notifications
You must be signed in to change notification settings - Fork 317
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
chore: remove cmake minium version deprecation warning #354
base: develop
Are you sure you want to change the base?
chore: remove cmake minium version deprecation warning #354
Conversation
We can also update to version 3.10 since this is what is suggested by the migration guide. |
22627ef
to
73d954d
Compare
@Maverobot, @marcbone I just fixed the latest CI errors, and I think we are now suitable for a merge 👍🏻. |
@rickstaa Thank you for your report and the fix. We will take a loot at this PR. |
73d954d
to
3829763
Compare
I rebased this PR onto the develop branch and added a CHANGELOG entry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good, but if possible remove the additions to models/
@@ -1,4 +1,4 @@ | |||
cmake_minimum_required(VERSION 3.1.3) | |||
cmake_minimum_required(VERSION 2.8.3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this not updated to 3.10 as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @gollth, thanks for reviewing my PR! I got an error when I updated it to 3.10 since the ROS meta package specs currently require version 2.8.3 (see https://wiki.ros.org/catkin/package.xml#Metapackages and moveit/moveit#1619).
How to reproduce
- Checkout 22627ef#diff-64225d9dae606e899687341a27336f3cfff2889d702f52095a148a4e6d50cb24 or change the CMake version in the
franka_ros/CMakeLists.txt
file toVERSION 3.10
. - Use
catkin_make
to build the package. - Be greeted by the following error:
WARNING: The CMakeLists.txt of the metapackage 'franka_ros' contains non standard content. Use the content of the following file instead: /home/ricks/development/work/franka_ros_ws/build/catkin_generated/metapackages/franka_ros/CMakeLists.txt
Note
Catkin-tools doesn't have this problem.
3829763
to
a37aee8
Compare
This commit ensure that the cmake minimum version < 3.5 deprecation warning is no longer thrown. The minimum cmake version was updated to 3.10 as this is the version that is recommended by the [ROS noetic migration guide](https://wiki.ros.org/noetic/Migration#Increase_required_CMake_version_to_avoid_author_warning). The meta package cmake version has to be kept on 2.8.3 because of some ROS requirements (see https://wiki.ros.org/catkin/package.xml#Metapackages).
a37aee8
to
ca0394f
Compare
Not sure if this is desirable given back-compatibility, but this pull request ensures that the cmake minimum version < 3.5 deprecation warning is no longer thrown: