-
Notifications
You must be signed in to change notification settings - Fork 88
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
Move Tesseract_ros to its own package #207
Comments
I think that the packages are fully separated now. When I build Tesseract for ROS2 I just put a |
I came across this repository showing an example having ros1 and ros2 packages in the same repository. I created a PR showing how to merge them into a single package so the package name does not change between versions of ros. I wanted to get your thoughts on using this approach to merge in tesseract_ros2 into this repository? @schornakj @mpowelson @marip8 @jrgnicho @jdlangs |
@Levi-Armstrong your proposed changes there look fine to me, although just adding a COLCON_IGNORE file would be much simpler and it would still build under ROS1 as long as tesseract_ros2 remains in its own repo |
Not using the ugly conditionals in package manifests would also be much less .. well .. ugly ;) |
Currently there are three options.
I would like to get everyone's opinion about which one they prefer. @gavanderhoorn @schornakj @mpowelson @marip8 @jrgnicho @jdlangs |
I think my preference would be first Option 1. Then Option 3. Option 1 allows non ROS users to simply not clone the unused repositories rather than having them there but not building them. Option 3 is easy and I suppose is a bit more convenient. However, I'm already imagining either myself or code following in an IDE getting confused and changing the wrong headers. Plus, I would think in a year or two a large percentage of our projects will be in ROS2 making this structure more inconvenient than useful. Though I suppose we could just restructure to remove ROS1 at that point. I'm not sure I have any reasons for not liking 2. It just seems messy to add 2 to the package name. Do any of these options make a future release easier/harder? |
I posted a (very) long comment this (my) morning, but for some reason it seems to have disappeared. :( not sure what happened here. Edit: did any of you watching this issue/repository receive a github email notification about my comment? Edit2: summarising my comment from this morning (perhaps I was hallucinating): I favour alternative 1. No confusion, no ugly conditionals in package manifests, no work-arounds in build scripts, no potential confusion with editors (good point @mpowelson) and only a minor amount of overhead. I had two general guidelines for when not to put things in the same repository:
I also had included a link to ROS2 Transition Strategy on ROS Discourse. |
@gavanderhoorn I didn't see an email on my end. |
I agree that option 1 seems to be the cleanest but as a developer I am pretty scared of having to coordinate changes that would affect both the core libraries and the wrappers across multiple repositories. To me, the core and wrappers are inherently a single project, and so it makes sense for them to live in a single repo where they will always be in sync and versioned together. So I actually prefer option 2 here. I also don't like putting the number 2 on package names though; my suggestion would be to use names like |
One argument against this is that we separate "core libraries" and wrappers exactly because we don't see them as "a single project", but a library with business logic and some other entity that makes that available in the context of a ROS node graph (ie: a wrapper node). |
Business logic changes are one thing but wouldn't any change to the client interface / API of the core require a corresponding change in the wrapper? Are they not tightly coupled at that layer? |
O wait, I interpreted your comment as inverted to what you're writing.
They are certainly coupled. But only in one way: the wrapper depends on the core library. Not the other way around. If core > wrapper, something has gone rather wrong. |
Definitely the one-way dependency is important. My point is that if the dependency is such that if changes to the interface of the core require changes to the wrapper, it strongly suggests all those changes should be happening in a unified way. Admittedly, I haven't dug much into a project that uses a structure like this so I don't have much intuition about how often this is the case. |
@jdlangs and @gavanderhoorn Thank you for the discussion. I personally have struggled with the two option discussed. Keeping everything in one repository is nice from a maintains perspective, but from a developers perspective it is nice to keep them isolated. I am still not sure what is the best way forward is. I think we will keep down the original path of isolating them in separate repositories and see how things go. |
As of #241 this repo doesn't depend on ROS. We now have essentially 4 repos
|
I know this has been discussed, and I believe there are some roadblocks to this. What are they and how do we plan on resolving them?
The text was updated successfully, but these errors were encountered: