Backport: Don't include the gz/math.hh header from library code (#1043) #1044
Conversation
Using overly-broad include statements leads to slower builds and bloated object code. Signed-off-by: Jeremy Nimmer <[email protected]>
github-actions
bot
added
Gazebo 1️1️
|
we should make sure that gazebo11 and ignition-citadel packages still compile with this change |
Codecov Report
@@ Coverage Diff @@
## sdf9 #1044 +/- ##
===========================================
+ Coverage 43.47% 87.59% +44.11%
===========================================
Files 1 64 +63
Lines 23 10061 +10038
===========================================
+ Hits 10 8813 +8803
- Misses 13 1248 +1235
Continue to review full report at Codecov.
|
Good point. I tested with citadel and there were some build failures in gazebo (gz-sim). Should I fix those or is this change not backportable? |
let's start by fixing any bugs we find, and then we can decided on whether to backport |
|
gazebo11 needs some changes: gazebosim/gazebo-classic#3227 |
| #include <ignition/math/Pose3.hh> | ||
| #include <ignition/math/Quaternion.hh> | ||
| #include <ignition/math/Vector2.hh> | ||
| #include <ignition/math/Vector3.hh> |
There was a problem hiding this comment.
Even though this is the right thing to do, we should minimize the chances of breaking downstream users on a stable version. That's why we decided to target gazebosim/gz-transport#315 at Garden for example - that specific include change will definitely break lots of users. Here it could be argued that Param.hh is not as widely used as Node.hh, but I think the same principle applies.
|
this may break downstream code, so let's skip it |
Backport of #1043 with additional changes for the
ignition->gzrename.Note to maintainers: Remember to Merge with commit (not squash-merge or rebase)
🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸