-
Notifications
You must be signed in to change notification settings - Fork 322
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # 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: