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

Review casts in PCL, Part B #5508

Merged
merged 12 commits into from
Jan 17, 2023
Merged

Review casts in PCL, Part B #5508

merged 12 commits into from
Jan 17, 2023

Conversation

gnawme
Copy link
Contributor

@gnawme gnawme commented Nov 8, 2022

A follow-on to #5504, this PR introduces google-readability-casting to clang-tidy to convert C-style casts to C++ casts, and remove many redundant casts (casts to same type)

larshg
larshg previously approved these changes Nov 11, 2022
View reviewed changes
mvieth
mvieth reviewed Nov 13, 2022
View reviewed changes
Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

30 files reviewed so far

io/include/pcl/io/grabber.h Outdated Show resolved Hide resolved
features/include/pcl/features/impl/integral_image_normal.hpp Outdated Show resolved Hide resolved
common/src/PCLPointCloud2.cpp Outdated Show resolved Hide resolved
@gnawme
Copy link
Contributor Author

gnawme commented Dec 14, 2022

@mvieth Were you waiting for me to do something else on this PR?

@mvieth
Copy link
Member

mvieth commented Dec 15, 2022 •
edited
Loading

@mvieth Were you waiting for me to do something else on this PR?

No, it is just that I am short on time and the PR is quite large, so it takes a bit until I am done 🙂 (I have reviewed 70 files so far)

Edit: actually now there are conflicts, please resolve them

@gnawme gnawme force-pushed the norm.evangelista/review-casts-part-b-PCL-4230 branch from 2a48b16 to 865ecd9 Compare December 17, 2022 22:28
mvieth
mvieth reviewed Dec 23, 2022
View reviewed changes
Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

Reviewed 100/161 files so far

geometry/include/pcl/geometry/mesh_base.h Outdated Show resolved Hide resolved
simulation/tools/sim_viewer.cpp Outdated Show resolved Hide resolved
@gnawme gnawme force-pushed the norm.evangelista/review-casts-part-b-PCL-4230 branch from 865ecd9 to 5175ecc Compare December 24, 2022 07:19
mvieth
mvieth reviewed Jan 4, 2023
View reviewed changes
Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

140/161 files reviewed

registration/src/correspondence_rejection_trimmed.cpp Outdated Show resolved Hide resolved
common/src/PCLPointCloud2.cpp Outdated Show resolved Hide resolved
@gnawme gnawme force-pushed the norm.evangelista/review-casts-part-b-PCL-4230 branch from 700126b to 5107383 Compare January 12, 2023 17:54
mvieth
mvieth reviewed Jan 16, 2023
View reviewed changes
Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

Last comments, otherwise the changes look good.

geometry/include/pcl/geometry/mesh_base.h Outdated Show resolved Hide resolved
geometry/include/pcl/geometry/mesh_base.h Outdated Show resolved Hide resolved
mvieth
mvieth approved these changes Jan 17, 2023
View reviewed changes
Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

Thank you!
For future pull requests: please make sure that they are smaller, so reviewing doesn't take as long and they can be merged faster.
@larshg Since you previously approved this PR, I am going to assume that you still do

@mvieth mvieth merged commit b390826 into PointCloudLibrary:master Jan 17, 2023
@gnawme gnawme deleted the norm.evangelista/review-casts-part-b-PCL-4230 branch January 17, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvieth mvieth mvieth approved these changes

@larshg larshg larshg left review comments

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@gnawme @mvieth @larshg