strands-project / mongodb_store

MongoDB tools for storing and analysing runs of ROS systems.
BSD 3-Clause "New" or "Revised" License
49 stars 72 forks source link

if compiler supports it use C++11 #223

Closed bbferka closed 6 years ago

bbferka commented 6 years ago

added compilation flags for C++11

furushchev commented 6 years ago

@bbferka Thank you for sending a pull request! Just one question, is it possible to enable/disable c++11 features using something like if($ENV{ROS_DISTRO}) condition? I think it'd be the best if it is possible to disable this feature on indigo.

bbferka commented 6 years ago

@furushchev I guess that is also possible. Since indigo only works with Ubuntu 14.04, which comes with an old gcc, check_cxx_compiler_flag("-std=c++11" COMPILER_SUPPORTS_CXX11) would also simply result in building without c++11 for indigo; If you prefer the ROS_DISTRO solution I can change that.

furushchev commented 6 years ago

@bbferka Cool! I think it is enough just to add it as a comment to CMakeLists.txt that c++11 feature will be disabled in indigo.

bbferka commented 6 years ago

@furushchev sorry, I was mistaken. gcc 4.8 already has C++11 support. I will change my code to use the ROS_ENV env. var., and disable C++11 for indigo

hawesie commented 6 years ago

What about future releases? The current setup means we have to manually add each new ROS release that supports C++11. Wouldn't it be better to check the negative, e.g. if indigo then c++11 is disabled, else all other releases have it enabled?

bbferka commented 6 years ago

@hawesie you're right. I just changed the check;

hawesie commented 6 years ago

Tests pass. All is well. Thanks @bbferka

furushchev commented 6 years ago

@bbferka @hawesie Thanks too for interating from me 👍

furushchev commented 6 years ago

@hawesie Could you release the new version including this change? I think it is not a small change, so recommend to increment a major or minor version (more than usual updates).

hawesie commented 6 years ago

ok, it's on my list for today

bbferka commented 6 years ago

@furushchev @hawesie thanks for merging!

hawesie commented 6 years ago

Released: