Skip to content
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

Use rpyutils import_c_library function for rosbag2_transport_py #482

Merged
merged 1 commit into from
Aug 7, 2020

Conversation

emersonknapp
Copy link
Collaborator

Use rpyutils import_c_library function for rosbag2_transport_py - instead of re-defining the logic here (with outdated information)

Depends on ros2/rpyutils#4
Closes #445

…thon

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp
Copy link
Collaborator Author

emersonknapp commented Aug 5, 2020

using https://gist.githubusercontent.com/emersonknapp/03048af0c22cf44348e76a3da1e1dd1b/raw/5fc3c523f2e6446a0d7e941d7cff9d90c13f4eaa/ros2.repos

  • Linux Build Status

  • Linux-aarch64 Build Status

  • macOS Build Status

  • Windows Build Status

  • Windows (re-triggered to check for flaky test) Build Status

  • Windows Debug Build Status

Since library loading can sometimes misbehave on Windows Debug, added a CI run specifically testing it.

@emersonknapp emersonknapp merged commit f1d1145 into master Aug 7, 2020
@Karsten1987
Copy link
Collaborator

@emersonknapp this branch can be deleted, I think.

@Karsten1987
Copy link
Collaborator

Also, the URL should be corrected in older/existing distros. I guess this PR can't really be used to be backported, can it?

@emersonknapp
Copy link
Collaborator Author

Ah - this I didn't think all the way through - we could backport this only into Foxy because rpyutils was added in that distro.

What do you think about a single backport patch commit to Foxy, Eloquent, and Dashing that just copies the contents of the new rpyutils function?

@Karsten1987
Copy link
Collaborator

What do you think about a single backport patch commit to Foxy, Eloquent, and Dashing that just copies the contents of the new rpyutils function?

I think it makes sense to backport this PR to Foxy. For Eloquent and Dashing I would actually really just change the URL.

emersonknapp added a commit that referenced this pull request Aug 7, 2020
…thon (#482)

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
emersonknapp added a commit that referenced this pull request Aug 19, 2020
…thon (#482) (#494)

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@jaisontj jaisontj deleted the emersonknapp/rpyutils-import-c-library branch October 20, 2020 21:18
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.

fix troubleshooting URL
4 participants