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.

Sign up for GitHub

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

Jump to bottom

Enable YAML encoding/decoding for RecordOptions and StorageOptions #916

Merged
merged 2 commits into from
Nov 24, 2021
Merged

Enable YAML encoding/decoding for RecordOptions and StorageOptions #916

merged 2 commits into from
Nov 24, 2021

Conversation

emersonknapp
Copy link
Collaborator

@emersonknapp emersonknapp commented Nov 23, 2021 •
edited
Loading

Related to #831

Initial: Create a common yaml.hpp header so that we stop copy-pasting the same block of windows workarounds.

Add YAML codec for RecordOptions and StorageOptions structs, this will be used for input for bag converter.

@emersonknapp emersonknapp force-pushed the emersonknapp/storage-record-options-yaml-codec branch 2 times, most recently from b44d6e8 to eea557a Compare November 23, 2021 20:50
Enable YAML encoding/decoding for RecordOptions and StorageOptions
a79e01f 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp emersonknapp force-pushed the emersonknapp/storage-record-options-yaml-codec branch from eea557a to a79e01f Compare November 23, 2021 21:04
Add a simple codec test for both structs
212575b 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp emersonknapp marked this pull request as ready for review November 23, 2021 21:46
@emersonknapp emersonknapp requested a review from a team as a code owner November 23, 2021 21:46
@emersonknapp emersonknapp requested review from gbiggs, hidmic and lihui815 and removed request for a team November 23, 2021 21:46
lihui815
lihui815 approved these changes Nov 23, 2021
View reviewed changes
Copy link
Contributor

@lihui815 lihui815 left a comment

Choose a reason for hiding this comment

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

lgtm

@emersonknapp
Copy link
Collaborator Author

Gist: https://gist.githubusercontent.com/emersonknapp/432591c24299effade0328706a05c47a/raw/f7a45e5f9feda79cb741fb9e341372b819c566cd/ros2.repos
BUILD args: --packages-up-to rosbag2_performance_benchmarking rosbag2_storage rosbag2_storage_default_plugins rosbag2_transport rosbag2_tests rosbag2
TEST args: --packages-select rosbag2_performance_benchmarking rosbag2_storage rosbag2_storage_default_plugins rosbag2_transport rosbag2_tests rosbag2
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/9383

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@emersonknapp emersonknapp merged commit 76a39fd into master Nov 24, 2021
@delete-merged-branch delete-merged-branch bot deleted the emersonknapp/storage-record-options-yaml-codec branch November 24, 2021 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lihui815 lihui815 lihui815 approved these changes

@gbiggs gbiggs Awaiting requested review from gbiggs gbiggs is a code owner automatically assigned from ros2/tooling-reviewers

@hidmic hidmic Awaiting requested review from hidmic hidmic is a code owner automatically assigned from ros2/tooling-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@emersonknapp @lihui815