-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add in ros_python_version. #4
Conversation
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
CMakeLists.txt
Outdated
@@ -7,6 +7,13 @@ catkin_package() | |||
set(ROS_VERSION "1") | |||
set(ROS_DISTRO "melodic") | |||
|
|||
set(ROS_PY_VER "$ENV{ROS_PYTHON_VERSION}") | |||
if(ROS_PY_VER) | |||
set(ROS_PYTHON_VERSION "${ROS_PY_VER}") |
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.
I would suggest to change this to the following to avoid the temporary variable:
if(DEFINED ENV{VAR})
set(ROS_PYTHON_VERSION "$ENV{ROS_PYTHON_VERSION}")
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.
Good call, done in ecffcd0
I think this should be added to REP 149 too. |
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Will do. |
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.
LGTM.
….em (#13) * Update env. hook batch file to set ros_package_path correctly (#1) * Get ROS_PACKAGE_PATH from the python (#2) * Get ROS_PACKAGE_PATH from the python * normcase workspace path and eliminate duplicate entries * Preserve the content from CMAKE_PREFIX_PATH and remove duplicate entries (#4) * use set to remove duplicate entries in a list
Signed-off-by: Chris Lalancette clalancette@openrobotics.org