Move smach_tutorials (from https://github.com/eacousineau/executive_smach_tutorials.git). Catkinized.#19
Move smach_tutorials (from https://github.com/eacousineau/executive_smach_tutorials.git). Catkinized.#19130s wants to merge 5 commits intoros:indigo-develfrom
Conversation
|
@eacousineau I also appreciate your opinion. |
|
@rhaschke @spmaniato After I opened this PR I found you guys made some changes in your forked repos, so I merged them in here. I appreciate if you have any comments. |
|
Thanks Isaac for taking the initiative on this. |
|
Late to the game, but I'd certainly be fine with this making it into the common tutorials as long as the Wiki only points to one repo (maybe mentioning the other as a mirror) - a concern is that it may be confusing if two copies of code are referenced, especially if diverge at any point. Thanks! |
|
@stonier or any supervising mainteners, could you give a review? With a lack of central place for |
Thanks for the comment. I can certainly update any duplicate/confusing info once this PR is merged in. |
|
I'm a little ambivalent about having smach here as it's not really a common part of any ROS system whereas actionlib, nodelets/pluginlib are, but I don't have a strong objection either - if it's useful, great. Technically it doesn't introduce an undesirable dependency, particularly with respect to where it lands for the @clalancette since you're overseeing melodic, do you have any objections? The alternative would be to drop it into its own repository. If it does go in, two things:
|
|
@stonier I only care if it adds new dependencies to desktop/desktop_full. In this case, it looks like it adds the following new dependencies to the packages in this repository:
|
…mach_tutorials.git). Catkinized. According to the repo that currently hosts `executive_smach_tutorials` mentioned above, which is also referenced in the [tutorial set of SMACH](http://wiki.ros.org/smach/Tutorials), the code was evacuated from https://code.ros.org/svn/ros-pkg/stacks/executive_smach_tutorials/trunk fortunately before the website went unaccessible. Since `executive_smach` is part of core ROS (hosted under `ros` org https://github.com/ros/executive_smach), I think it makes sense to move this tutorial package to also `ros` org. The package is also catkinized this time.
|
Thanks for the review. Could we move on merging (& possibly making a new release)? In response to @stonier's comments, which I think are addressed:
It's exec_depend-ed.
Done ddfc8bc. Locally confirmed that all python scripts in the package are installed: |
|
Friendly ping @clalancette @stonier |
|
@clalancette @stonier Could we please have progress on this? I just was contacted by users (again) who are confused by the large number of forks of this tutorial repository. Would be really nice, to move this into a central place asap. |
clalancette
left a comment
clalancette
left a comment
There was a problem hiding this comment.
This looks generally fine to me, with a small nit in the CMakeLists.txt. I'll approve anyway. It's up to @stonier to merge it, as he is the maintainer here.
|
Critical Just tested the executive scripts, they no longer work as is in melodic. Looks like things have moved around in smach. e.g. Nice-to-have Rosrunnables instead of scripts in the share directory. That would match what the other packages in this repo are doing. |
5 participants
According to the repo that currently hosts
executive_smach_tutorialsmentioned above, which is also referenced in the tutorial set of SMACH, the code was evacuated from https://code.ros.org/svn/ros-pkg/stacks/executive_smach_tutorials/trunk fortunately before the website went unaccessible.Since
executive_smachis part of core ROS (hosted underrosorg https://github.com/ros/executive_smach), I think it makes sense to move this tutorial package to alsorosorg.The package is also catkinized this time.