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

[backport foxy-future] Reindexer (#641, #699) #860

Closed

Conversation

emersonknapp
Copy link
Collaborator

@emersonknapp emersonknapp commented Aug 31, 2021

Backport PRs #641 (reindexer functionality) and #699 (reindexer CLI) to foxy-future branch.

Related to #859

jhdcs added 2 commits August 31, 2021 15:17
Add a new C++ Reindexer class for reconstructing metadata from bags that are missing it.

Distro A, OPSEC #4584
Signed-off-by: Jacob Hassold <[email protected]>
Signed-off-by: Emerson Knapp <[email protected]>
* CLI interface to the rosbag2_cpp Reindexer `ros2 bag reindex`
Distro A, OPSEC #4584

Signed-off-by: Jacob Hassold <[email protected]>
@emersonknapp emersonknapp requested a review from a team as a code owner August 31, 2021 22:20
@emersonknapp emersonknapp requested review from Karsten1987 and MichaelOrlov and removed request for a team August 31, 2021 22:20
@emersonknapp emersonknapp force-pushed the emersonknapp/backport-foxy-future-reindexer branch 2 times, most recently from dd5ce35 to 98ff023 Compare August 31, 2021 23:27
@emersonknapp
Copy link
Collaborator Author

Compatibility fixes in separate commit:

  • rcutils Directory iteration didn't exist yet, so copy-pasted that implementation in-place for the backport.
  • rcpputils::path operator / didn't allow for const, so worked around that
  • No << operator for path, worked around that
  • Added linter skipping for the C code which is not from the same standard

Signed-off-by: Emerson Knapp <[email protected]>
@emersonknapp emersonknapp force-pushed the emersonknapp/backport-foxy-future-reindexer branch from 98ff023 to fb273e3 Compare September 1, 2021 00:12
Copy link
Contributor

@MichaelOrlov MichaelOrlov left a comment

Choose a reason for hiding this comment

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

While implementation looks god for me, I would recommend to move compatibility related code which is corresponds to the ros2/rcutils#323 to the separate files in the same rosbag2_cpp include and src folders.
The rational for that is that if someone in the future will try to backport ros2/rcutils#323 the code will not compile until removing this workaround. And it will not pollute reindexer.cpp

@EsipovPA
Copy link
Contributor

Is there some way to fix one last failed check? Looks like it is some kind of problem with signing-off commits

@emersonknapp
Copy link
Collaborator Author

That's not a blocking issue - more importantly it needs ci.ros2.org run on it

@emersonknapp
Copy link
Collaborator Author

emersonknapp commented Sep 24, 2021

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

@MichaelOrlov MichaelOrlov deleted the emersonknapp/backport-foxy-future-reindexer branch July 13, 2024 07:08
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.

4 participants