Skip to content

Conversation

j-rivero
Copy link
Contributor

Move dartsim/World.hh header out from public headers to be in src/ as a private header as suggested long ago in #274 (comment)

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

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.

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Sep 25, 2023
@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

Merging #551 (8961281) into gz-physics7 (9165154) will not change coverage.
Report is 1 commits behind head on gz-physics7.
The diff coverage is n/a.

❗ Current head 8961281 differs from pull request most recent head ab38813. Consider uploading reports for the commit ab38813 to get more accurate results

@@             Coverage Diff              @@
##           gz-physics7     #551   +/-   ##
============================================
  Coverage        76.89%   76.89%           
============================================
  Files              140      140           
  Lines             7683     7683           
============================================
  Hits              5908     5908           
  Misses            1775     1775           
Files Coverage Δ
dartsim/src/World.hh 100.00% <ø> (ø)

@azeey
Copy link
Contributor

azeey commented Sep 26, 2023

After this PR, would the libgz-physics7-dartsim-dev package become obsolete?

@scpeters
Copy link
Member

After this PR, would the libgz-physics7-dartsim-dev package become obsolete?

that still holds the cmake config files, so I think it's needed?

@j-rivero
Copy link
Contributor Author

After this PR, would the libgz-physics7-dartsim-dev package become obsolete?

that still holds the cmake config files, so I think it's needed?

* https://github.com/gazebo-release/gz-physics7-release/blob/main/ubuntu/debian/libgz-physics-dartsim-dev.install 

that still holds the cmake config files, so I think it's needed?

This is a good point. I think that the main question is: is there a valid use for developers that would generate code from the dartsim plugin that has no public headers? Because we are generating the buildsystem helpers (CMake + pkgconfig) to facilitate that use case. If there is no use case, we either not packaging the helpers and remove the -dev package or change our buildsystem not to generate the helpers.

@j-rivero j-rivero merged commit 578b4ed into gz-physics7 Sep 26, 2023
@j-rivero j-rivero deleted the jrivero/dartsim_header_move branch September 26, 2023 21:39
@j-rivero
Copy link
Contributor Author

j-rivero commented Sep 26, 2023

Let's move the conversation to #541

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants