Skip to content

[WIP] rclcpp design#2

Closed
Theosakamg wants to merge 31 commits intoros2-java:masterfrom
ros2java-alfred:master
Closed

[WIP] rclcpp design#2
Theosakamg wants to merge 31 commits intoros2-java:masterfrom
ros2java-alfred:master

Conversation

@Theosakamg
Copy link

@esteve
Copy link
Member

esteve commented Nov 13, 2016

@Theosakamg @elehuitouze thanks for your contributions, they are great!

As I asked @Theosakamg on his ros2_java pull request (https://github.com/esteve/ros2_java/pull/8#issuecomment-257613110 and https://github.com/esteve/ros2_java/pull/8#issuecomment-260190111), I would really prefer if you created separate pull requests for each improvement, I can't merge this pull request as is.

One particular thing is that this pull request replaces the Gradle plugin with your own. It'd be great if you submitted any improvements to the Gradle plugin I created (https://github.com/esteve/ament_gradle_plugin), otherwise it's going to confuse users as to which Gradle plugin to use. Moreover, what features were missing in my Gradle plugin? I had a quick look at yours and it does not work with Android, and the support for Eclipse in your plugin is broken. If you want to add support for Eclipse, you only need to modify the build.gradle file in a Gradle project to add apply plugin: 'eclipse' (https://docs.gradle.org/current/userguide/eclipse_plugin.html#sec:eclipse_usage), so I don't fully understand why you built that into your Gradle plugin.

@esteve
Copy link
Member

esteve commented Nov 20, 2016

@Theosakamg @elehuitouze so, any update? No response from you guys since I ask you to split it into concrete pull requests, so I'm thinking of closing this pull request in the following days if I don't hear from you.

@Theosakamg
Copy link
Author

@esteve Sorry for delay. the PR is WIP.

@Theosakamg Theosakamg changed the title rclcpp design [WIP] rclcpp design Nov 20, 2016
@Theosakamg Theosakamg closed this Nov 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants