Conversation
|
Any changes required here? |
clalancette
left a comment
There was a problem hiding this comment.
The basic idea is fine, but there are some details that need fixing. Also, this will need to be rebased and conflicts fixed since the ros2 branch is branched off of melodic-devel, not indigo-devel.
| <url type="repository">https://github.com/ros/urdf_parser_py</url> | ||
|
|
||
| <buildtool_depend>catkin</buildtool_depend> | ||
| <buildtool_depend>ament_cmake</buildtool_depend> |
There was a problem hiding this comment.
From what I can tell looking elsewhere, with a pure python package we don't need any buildtool_depend line, so just remove this.
There was a problem hiding this comment.
This comment is still open.
| ) | ||
|
|
||
| setup(**d) | ||
| # from distutils.core import setup |
There was a problem hiding this comment.
Just delete all of this commented code.
| 'Topic :: Software Development', | ||
| ], | ||
| description='Python implementation of the URDF parser.', | ||
| license='Apache License, Version 2.0', |
There was a problem hiding this comment.
This has to stay BSD, since we aren't really changing the underlying code.
| d = generate_distutils_setup( | ||
| setup( | ||
| name=package_name, | ||
| version='0.3.3', |
There was a problem hiding this comment.
melodic-devel is currently on 0.4.0, but since this is going to be a new branch, we probably want to bump up to something like 1.0.0 here (and in the package.xml).
| zip_safe=True, | ||
| author='Víctor Mayoral Vilches', | ||
| author_email='vmayoral@acutronicrobotics.com', | ||
| maintainer='Víctor Mayoral Vilches', |
There was a problem hiding this comment.
I don't mind adding you as a maintainer, but if you want to be the maintainer I'd suggest that we add you to the package.xml as well. If you don't want to be a maintainer, you can just put my name/email here.
Fixes bug in xml_string method and updates rosdeps for ROS2
Remove non-ascii characters from setup config
| <url type="repository">https://github.com/ros/urdf_parser_py</url> | ||
|
|
||
| <buildtool_depend>catkin</buildtool_depend> | ||
| <buildtool_depend>ament_cmake</buildtool_depend> |
There was a problem hiding this comment.
This comment is still open.
| xmlString = etree.tostring(rootXml, pretty_print=True) | ||
| xmlString = etree.tostring(rootXml, pretty_print=True, encoding=str) | ||
| if addHeader: | ||
| xmlString = '<?xml version="1.0"?>\n' + xmlString |
There was a problem hiding this comment.
Any particular reason for this change? I think this can stay as-is.
|
Closing in favor of #53 |
|
This pull request has been mentioned on ROS Discourse. There might be relevant details there: https://discourse.ros.org/t/the-moveit-2-journey-part-1-porting-and-understanding-moveit-core/8718/1 |
Note to maintainers: Needs to be merged in a new
ros2branch.