search

moveit / moveit_resources

URDFs, meshes, and config packages for MoveIt testing
61 stars 114 forks source link

[ROS-O] do not enforce "old" standard #155

Closed v4hn closed 1 year ago

v4hn commented 1 year ago

C++14 is default in clang/gcc anyway and new log4cxx requires C++17.

It's better not to declare it at all and leave the choice to the system environment.

required for ROS-O.

v4hn commented 1 year ago

As moveit_resources is used also in Bionic, we still need the CXX_STANDARD definition there.

I don't see the need? c++14 is default in gcc from 6.1 and ubuntu bionic packages 7.4? I just validated that in the ubuntu:18.04 docker image. The comment above the block you cite is simply wrong.

Jochen's snippet certainly is better than forcing the version unconditionally and it has the charm to force the compiler to check the standard strictly unless overwritten from the outside. That makes sense for MoveIt maintenance, but I don't really see the need for our "special case" prbt_ikfast_manipulator_plugin in moveit_resources. If you insist we should probably switch to the macro from moveit_core/cmake as well.

But C++14 has been the default for a really long time now I don't see the need for more logic over no logic here unless that code starts using c++17/20/23 features.