Skip to content

Move geometry2 (both locally, and on github).#324

Merged
clalancette merged 2 commits intomasterfrom
geometry2-fork
Mar 24, 2017
Merged

Move geometry2 (both locally, and on github).#324
clalancette merged 2 commits intomasterfrom
geometry2-fork

Conversation

@clalancette
Copy link
Copy Markdown
Contributor

We want to switch to using our fork, instead of a branch on
the original. This matches what we do for other repositories.

Signed-off-by: Chris Lalancette clalancette@osrfoundation.org

We want to switch to using our fork, instead of a branch on
the original.  This matches what we do for other repositories.

Signed-off-by: Chris Lalancette <clalancette@osrfoundation.org>
@clalancette clalancette added the in progress Actively being worked on (Kanban column) label Mar 22, 2017
ros2.repos Outdated
url: https://github.com/ros/class_loader.git
version: ros2
ros/geometry2:
ros2/geometry2:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: should be placed alphabetically in the list

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops. Done now.

Signed-off-by: Chris Lalancette <clalancette@osrfoundation.org>
@dhood
Copy link
Copy Markdown
Member

dhood commented Mar 22, 2017

do we have a standard yet for how we organise ported ros1-to-ros2 repos in this file? Was geometry2 in the ros directory because it was being pulled in from the ros github org, or because it is a ported ros1 package? Maybe we haven't really thought about it before, but at some point we should be decide so we can be consistent so that users know where to look for files

@mikaelarguedas
Copy link
Copy Markdown
Member

I think the logic was the folder name matches the org unit it's pulled from. But that's actually a guess based on observations, I don't know if we have a standard for that. It could make sense to dissociate the core of ros2 from the packages ported from ros1, at the folder level and even maybe at the org unit level

@dhood
Copy link
Copy Markdown
Member

dhood commented Mar 23, 2017

OK it's fine by me to match the org unit for now and we can always change it in the future if it makes more sense to have a different structure for ported packages 👍

Copy link
Copy Markdown
Member

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@clalancette clalancette added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Mar 24, 2017
@clalancette clalancette merged commit ba33157 into master Mar 24, 2017
@clalancette clalancette removed the in review Waiting for review (Kanban column) label Mar 24, 2017
@clalancette clalancette deleted the geometry2-fork branch March 24, 2017 19:29
@clalancette
Copy link
Copy Markdown
Contributor Author

Thanks all, merged now. I think this won't happen automatically for people; should I send out a notification somewhere that the location has changed?

@mikaelarguedas
Copy link
Copy Markdown
Member

Any change to the repos file will require people to pull the new one and import it in their workspace. You are right that people already having geometry2 in their workspace will have a hiccup of duplicate packages (ament will notify the duplication and the location of the conflicting packages).
@ros2/team please think about removing the current version of geometry2 in the ros subfolder before pulling this updated repos file.

Jiusi-pys pushed a commit to Jiusi-pys/ros2 that referenced this pull request Jan 17, 2026
* Move geometry2 (both locally, and on github).

We want to switch to using our fork, instead of a branch on
the original.  This matches what we do for other repositories.

Signed-off-by: Chris Lalancette <clalancette@osrfoundation.org>
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.

3 participants