[ROS 2][grid_map_core] Code enhancements from master b7293f5 #494
Conversation
[grid map core] Reduce clang warnings GitOrigin-RevId: 0b01ff34cd31833b65f4718fab82467f11332938
|
Can you post the clang warnings this solves? Note that the change of the type to a function and the removal of |
| @@ -30,14 +30,14 @@ class CircleIterator | |||
| * @param center the position of the circle center. | |||
| * @param radius the radius of the circle. | |||
| */ | |||
| CircleIterator(const GridMap & gridMap, const Position & center, const double radius); | |||
There was a problem hiding this comment.
I would like know what the motivation is for this change. It goes against const correctness.
https://isocpp.org/wiki/faq/const-correctness
Can you share what compile warning this solves?
There was a problem hiding this comment.
Ah to be honest I didn't get these warnings myself. The title is a bit confusing, but this was just part of the effort to make rolling as up to date as possible as master.
Is it not advisable to fix these if we're not seeing them ourselves during colcon build? Then I would just port the major changes from these, but it's a bit confusing to know what was a needed fix and what was just a code clean up.
There was a problem hiding this comment.
Yea, especially if the change breaks ABI, I don't see any strong reason to fold it in. Perhaps it's worth starting with any bugfixes that have associated tests?
|
One more thing - if this solves all of the clang warnings, it would be fantastic to add a CI job that enforces no new clang warnings. That would be a nice follow up to this. |
kjalloul-anybotics
changed the title
kjalloul-anybotics
changed the title
Cherry-picked commit b7293f5 (No PR)
Includes fixing typos,
math.htocmath,push_backtoemplace_back, and other code improvements.