-
Notifications
You must be signed in to change notification settings - Fork 39
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
Move geospatial headers to subfolder #289
Conversation
This simplifies the logic required for packaging. Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #289 +/- ##
=======================================
Coverage 76.69% 76.69%
=======================================
Files 76 76
Lines 10647 10647
=======================================
Hits 8166 8166
Misses 2481 2481
Continue to review full report at Codecov.
|
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, left a minor suggestion
Migration.md
Outdated
@@ -17,7 +17,8 @@ release will remove the deprecated code. | |||
1. `HeightmapData.hh` and `ImageHeightmap.hh` have been moved out of the | |||
`graphics` component and into the new `geospatial` component | |||
+ To use the heightmap features, users must add the `geospatial` component | |||
to the `find_package` call | |||
to the `find_package` call and update the include paths to use | |||
the geospatial subfolder (`#include <ignition/common/geospatial/Dem.hh>`) |
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.
Since Dem
is a new feature, how about we use ImageHeightmap.hh
as an example instead?
the geospatial subfolder (`#include <ignition/common/geospatial/Dem.hh>`) | |
the geospatial subfolder (e.g., `#include <ignition/common/geospatial/ImageHeightmap.hh>`) |
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.
I used HeightmapData.hh in c3d6053
Signed-off-by: Steve Peters <[email protected]>
the homebrew CI didn't build the |
This reverts commit e0a5490. Signed-off-by: Steve Peters <[email protected]>
* Revert "Move geospatial headers to subfolder (#289)" This reverts commit e0a5490. Signed-off-by: Steve Peters <[email protected]> * Revert "Geospatial component for heightmap & DEMs (#267)" This reverts commit 28eb6c7. Signed-off-by: Steve Peters <[email protected]>
This simplifies the logic required for packaging. Update Migration guide Signed-off-by: Steve Peters <[email protected]>
🦟 Bug fix
Follow-up to #267, connected to gazebo-release/gz-common5-release#4
Summary
This simplifies the logic required for packaging by putting all the geospatial header files in a subfolder. We haven't done this for the other ing-common components, but now is an easy time to do it for geospatial since we are already breaking things with this component.
Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.