Minor changes to indigo-devel CMake allow this to be used in kinetic and indigo #14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wkentaro
reviewed
Jan 20, 2017
| @@ -2,6 +2,11 @@ | |||
| cmake_minimum_required(VERSION 2.8.3) | |||
| project(robot_self_filter) | |||
|
|
|||
| if(CMAKE_COMPILER_IS_GNUCXX) | |||
| set(CMAKE_CXX_FLAGS "-std=c++11 ${CMAKE_CXX_FLAGS}") | |||
There was a problem hiding this comment.
Could you please explain why this is needed on kinetic?
|
I'm using kinetic and I need it???
Why would it not be needed on kinetic?
Is it folded into another package?
…On Thu, Jan 19, 2017 at 6:38 PM Kentaro Wada ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In CMakeLists.txt
<#14 (review)>
:
> @@ -2,6 +2,11 @@
cmake_minimum_required(VERSION 2.8.3)
project(robot_self_filter)
+if(CMAKE_COMPILER_IS_GNUCXX)
+ set(CMAKE_CXX_FLAGS "-std=c++11 ${CMAKE_CXX_FLAGS}")
Could you please explain why this is needed on kinetic?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#14 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADrfxjML9ppox188S5IgrTtaowGDbpr9ks5rUAH3gaJpZM4LdFnw>
.
|
|
Maybe I read your comment incorrectly. This particular fix is needed under
Kinetic because much of kinetic now uses C++-11 or higher. I'm not sure
exactly why, the you package doesn't compile under Kinetic without
explicitly stating C++-11 is needed. I've seen this in several other
packages as well.
…On Thu, Jan 19, 2017 at 6:42 PM Patrick Beeson ***@***.***> wrote:
I'm using kinetic and I need it???
Why would it not be needed on kinetic?
Is it folded into another package?
On Thu, Jan 19, 2017 at 6:38 PM Kentaro Wada ***@***.***>
wrote:
***@***.**** commented on this pull request.
------------------------------
In CMakeLists.txt
<#14 (review)>
:
> @@ -2,6 +2,11 @@
cmake_minimum_required(VERSION 2.8.3)
project(robot_self_filter)
+if(CMAKE_COMPILER_IS_GNUCXX)
+ set(CMAKE_CXX_FLAGS "-std=c++11 ${CMAKE_CXX_FLAGS}")
Could you please explain why this is needed on kinetic?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#14 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADrfxjML9ppox188S5IgrTtaowGDbpr9ks5rUAH3gaJpZM4LdFnw>
.
|
|
For example see ros-industrial/industrial_core#161
…On Thu, Jan 19, 2017 at 6:47 PM Patrick Beeson ***@***.***> wrote:
Maybe I read your comment incorrectly. This particular fix is needed under
Kinetic because much of kinetic now uses C++-11 or higher. I'm not sure
exactly why, the you package doesn't compile under Kinetic without
explicitly stating C++-11 is needed. I've seen this in several other
packages as well.
On Thu, Jan 19, 2017 at 6:42 PM Patrick Beeson ***@***.***> wrote:
I'm using kinetic and I need it???
Why would it not be needed on kinetic?
Is it folded into another package?
On Thu, Jan 19, 2017 at 6:38 PM Kentaro Wada ***@***.***>
wrote:
***@***.**** commented on this pull request.
------------------------------
In CMakeLists.txt
<#14 (review)>
:
> @@ -2,6 +2,11 @@
cmake_minimum_required(VERSION 2.8.3)
project(robot_self_filter)
+if(CMAKE_COMPILER_IS_GNUCXX)
+ set(CMAKE_CXX_FLAGS "-std=c++11 ${CMAKE_CXX_FLAGS}")
Could you please explain why this is needed on kinetic?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#14 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADrfxjML9ppox188S5IgrTtaowGDbpr9ks5rUAH3gaJpZM4LdFnw>
.
|
|
I wonder why this is not addressed on core packages like Or are there any information on roswiki? |
|
The newest compilers on 16.04 require c++-11 and higher to be explicitly
set for certain things. I don't recall what the exact compiler error that
made me figure out the c++-11 was needed, but it shouldn't be hard to
reproduce. I'll be back at my desk on Tuesday and can try then.
…On Thu, Jan 19, 2017 at 7:08 PM Kentaro Wada ***@***.***> wrote:
I wonder why this is not addressed on core packages like cv_bridge and
nodelet, which are already released to kinetic. Do you know why?
https://github.com/ros-perception/vision_opencv/blob/kinetic/cv_bridge/CMakeLists.txt
https://github.com/ros/nodelet_core/blob/indigo-devel/nodelet/CMakeLists.txt
Or are there any information on roswiki?
I could not find it on the migration page:
http://wiki.ros.org/kinetic/Migration
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#14 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADrfxlMrsjjlQDz34yNWlzONRiuoAaiuks5rUAkUgaJpZM4LdFnw>
.
|
|
Is this able to be merged and resolved? It would be nice to have this percolate through the 14.04 and 16.04 build farm and not have to keep a fork of your repo around anymore. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I still needed this package, despite the fact that its being released via the Debian packages anymore. I forked and made the simple changes in the CMakefile that allow this to run in 14.04 or 16.04 with Indigo, Jade, and Kinetic.