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
Add default initialization for CompressionOptions #1539
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.
minor comment, but needs to be resolved.
CC: @MichaelOrlov
rosbag2_compression/include/rosbag2_compression/compression_options.hpp
Outdated
Show resolved
Hide resolved
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.
@arneboe I have a minor concern about initial value for number of threads.
Also please fix DCO. See details in the https://github.com/ros2/rosbag2/pull/1539/checks?check_run_id=20499622275
Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com>
…tions.hpp Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com>
b3afed8
to
4de0d87
Compare
Done ;) |
@ros-pull-request-builder retest this please |
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.
LGTM
Gist: https://gist.githubusercontent.com/MichaelOrlov/d2aeb87a86a3fc667fbe88950d5f1ea8/raw/fd15696f0f6d091943f8f91903f39eb3682df54b/ros2.repos |
https://github.com/Mergifyio backport iron humble |
✅ Backports have been created
|
* feat: add sane defaults for CompressionOptions Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> * Update rosbag2_compression/include/rosbag2_compression/compression_options.hpp Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> --------- Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> Co-authored-by: Arne Böckmann <a.boeckmann@cellumation.com> Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> (cherry picked from commit 931bf54) # Conflicts: # rosbag2_compression/include/rosbag2_compression/compression_options.hpp
* feat: add sane defaults for CompressionOptions Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> * Update rosbag2_compression/include/rosbag2_compression/compression_options.hpp Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> --------- Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> Co-authored-by: Arne Böckmann <a.boeckmann@cellumation.com> Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> (cherry picked from commit 931bf54) # Conflicts: # rosbag2_compression/include/rosbag2_compression/compression_options.hpp
…1539) (#1546) * Add default initialization for CompressionOptions (#1539) * feat: add sane defaults for CompressionOptions Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> * Update rosbag2_compression/include/rosbag2_compression/compression_options.hpp Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> --------- Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> Co-authored-by: Arne Böckmann <a.boeckmann@cellumation.com> Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> (cherry picked from commit 931bf54) # Conflicts: # rosbag2_compression/include/rosbag2_compression/compression_options.hpp * Address merge conflicts Signed-off-by: Michael Orlov <michael.orlov@apex.ai> --------- Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Arne B <arne@rnae.de> Co-authored-by: Michael Orlov <michael.orlov@apex.ai>
) (#1545) * Add default initialization for CompressionOptions (#1539) * feat: add sane defaults for CompressionOptions Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> * Update rosbag2_compression/include/rosbag2_compression/compression_options.hpp Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> --------- Signed-off-by: Arne Böckmann <a.boeckmann@cellumation.com> Co-authored-by: Arne Böckmann <a.boeckmann@cellumation.com> Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> (cherry picked from commit 931bf54) # Conflicts: # rosbag2_compression/include/rosbag2_compression/compression_options.hpp * Address merge conflicts Signed-off-by: Michael Orlov <michael.orlov@apex.ai> --------- Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Arne B <arne@rnae.de> Co-authored-by: Michael Orlov <michael.orlov@apex.ai>
This PR adds sane defaults for the
CompressionOptions
struct.Sane defaults are important because the struct does not have a constructor and it is possible (and very easy) to use it without completely initializing it.
Another option would be to add a constructor and delete the default constructor, but that would not be backward compatible.