-
Notifications
You must be signed in to change notification settings - Fork 4
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 rosbag2 metapackage #16
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Signed-off-by: Zachary Michaels <zmichaels11@gmail.com>
166bb7d
to
d37e509
Compare
Signed-off-by: Zachary Michaels <zmichaels11@gmail.com>
rosbag2_bag_v2_plugins/src/rosbag2_bag_v2_plugins/converter/rosbag_v2_deserializer.cpp
Show resolved
Hide resolved
rosbag2_bag_v2_plugins/src/rosbag2_bag_v2_plugins/converter/rosbag_v2_deserializer.hpp
Show resolved
Hide resolved
Signed-off-by: Zachary Michaels <zmichaels11@gmail.com>
@Karsten1987 does this PR need CI to merge? |
The same comment applies here: ros2/rosbag2#254 (comment) |
Signed-off-by: Zachary Michaels <zmichaels11@gmail.com>
Building with ci_packaging Build status |
All tests passed on Linux CI, going to kick off CI for other OS's |
I ran CI with packaging on all OS.
@Karsten1987 what do you think should be done? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks good to me. For specifically this plugin, we don't have to care too much for MacOS and Windows as I doubt they every come out clean.
For the linux ones, the tests for the bag_v2
run and pass, so that's really what matters here IMO.
Changes
Blocked on ros2/rosbag2#253 since play e2e test is failing.