-
Notifications
You must be signed in to change notification settings - Fork 240
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
Remove rcpputils::fs dependencies from rosbag2_storages #1558
Conversation
Original author: Kenta Yonekura <yoneken@ieee.org> Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com>
Co-authored with Kenta Yonekura <yoneken@ieee.org> Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com>
@MichaelOrlov could you please take a look? |
@r7vme There are quite a few linter failures here. Please run the linters locally and fix the problems, then we can try again. |
Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com>
@clalancette fixed tests, PTAL |
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, but I'd like another opinion from @MichaelOrlov before we merge.
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.
@r7vme Thanks for the contribution. The changes looks good to me.
However, there is one open question for the future.
Do we really need to use the ".generic_string()" or simple .string()
would be enough?
Someone is going to have to test it out on Windows to tell for sure. |
As far as I understand from documentation the one should use |
https://github.com/Mergifyio backport iron humble |
✅ Backports have been created
|
* Remove rcpputils::fs dependencies from rosbag2_storages Original author: Kenta Yonekura <yoneken@ieee.org> Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> * Use path::generic_string() for Windows compatibility Co-authored with Kenta Yonekura <yoneken@ieee.org> Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> * Fix linter errors in osbag2_storage_sqlite3 Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> --------- Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> Co-authored-by: Kenta Yonekura <yoneken@ieee.org> (cherry picked from commit edda376) # Conflicts: # rosbag2_storage_mcap/test/rosbag2_storage_mcap/test_mcap_storage.cpp # rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp # rosbag2_storage_sqlite3/test/rosbag2_storage_sqlite3/test_sqlite_storage.cpp
* Remove rcpputils::fs dependencies from rosbag2_storages Original author: Kenta Yonekura <yoneken@ieee.org> Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> * Use path::generic_string() for Windows compatibility Co-authored with Kenta Yonekura <yoneken@ieee.org> Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> * Fix linter errors in osbag2_storage_sqlite3 Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> --------- Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> Co-authored-by: Kenta Yonekura <yoneken@ieee.org> (cherry picked from commit edda376) # Conflicts: # rosbag2_storage/CMakeLists.txt # rosbag2_storage_default_plugins/src/rosbag2_storage_default_plugins/sqlite/sqlite_storage.cpp # rosbag2_storage_default_plugins/test/rosbag2_storage_default_plugins/sqlite/test_sqlite_storage.cpp # rosbag2_storage_mcap/test/rosbag2_storage_mcap/test_mcap_storage.cpp
…ort #1558) (#1564) * Remove rcpputils::fs dependencies from rosbag2_storages (#1558) * Remove rcpputils::fs dependencies from rosbag2_storages Original author: Kenta Yonekura <yoneken@ieee.org> Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> * Use path::generic_string() for Windows compatibility Co-authored with Kenta Yonekura <yoneken@ieee.org> Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> * Fix linter errors in osbag2_storage_sqlite3 Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> --------- Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> Co-authored-by: Kenta Yonekura <yoneken@ieee.org> (cherry picked from commit edda376) # Conflicts: # rosbag2_storage_mcap/test/rosbag2_storage_mcap/test_mcap_storage.cpp # rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp # rosbag2_storage_sqlite3/test/rosbag2_storage_sqlite3/test_sqlite_storage.cpp * Resolve merge conflicts Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Update rosbag2_storage_sqlite3 to C++17. (#1501) Signed-off-by: Chris Lalancette <clalancette@gmail.com> (cherry picked from commit 9855072) --------- Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Roman <rsokolkov@gmail.com> Co-authored-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Original author: Kenta Yonekura yoneken@ieee.org
Copy of stalled PR #1474