Conversation
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #401 +/- ##
==========================================
- Coverage 47.89% 46.96% -0.93%
==========================================
Files 151 148 -3
Lines 12690 12174 -516
Branches 6703 6462 -241
==========================================
- Hits 6078 5718 -360
+ Misses 5259 5257 -2
+ Partials 1353 1199 -154
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
1de5141 to
fa7751e
Compare
|
@codex review: is it idiomatic, clean, maintainable, modern C++? Are there any behavior differences expected? Is implicit alignment of point structs enough to not break the ROS 2 pointcloud2 interface? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa7751e6d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/nebula_core/nebula_core_common/include/nebula_core_common/io/pcd.hpp
Show resolved
Hide resolved
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
drwnz
left a comment
There was a problem hiding this comment.
Nice, looks good - just some (optional) comments to look at.
src/nebula_core/nebula_core_common/include/nebula_core_common/io/pcd.hpp
Show resolved
Hide resolved
src/nebula_core/nebula_core_ros/include/nebula_core_ros/point_cloud_conversions.hpp
Outdated
Show resolved
Hide resolved
src/nebula_core/nebula_core_common/include/nebula_core_common/point_cloud.hpp
Outdated
Show resolved
Hide resolved
src/nebula_core/nebula_core_common/include/nebula_core_common/io/pcd.hpp
Outdated
Show resolved
Hide resolved
...ntinental/nebula_continental_common/include/nebula_continental_common/continental_srr520.hpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
🟢 Self-EvaluationBase functionality is covered by unit tests. 🟢 PointXYZIRCAEDT, PointXYZIR, PointXYZIRADTWhile PointXYZIRCAEDT worked out-of-the-box, the other two point types rely on conversion functions whose definition drifted apart from the header. Restored functionality in 183a297.
|

PR Type
Related Links
Description
PCL is packaged poorly on APT and even the dev dependency
libpcl-devintroduces 792 other APT packages as transitive dependenciesHere's the list (
apt-rdepends libpcl-dev| grep -E '^[a-zA-Z0-9]'):Dependency list
For the little part of PCL that we use, this needlessly slows down our CI.
Review Procedure
Remarks
Pre-Review Checklist for the PR Author
PR Author should check the checkboxes below when creating the PR.
Checklist for the PR Reviewer
Reviewers should check the checkboxes below before approval.
Post-Review Checklist for the PR Author
PR Author should check the checkboxes below before merging.
CI Checks