Skip to content

Option to install a header, but don't include it in master header#234

Closed
chapulina wants to merge 1 commit intoign-cmake2from
chapulina/2/install_include
Closed

Option to install a header, but don't include it in master header#234
chapulina wants to merge 1 commit intoign-cmake2from
chapulina/2/install_include

Conversation

@chapulina
Copy link
Contributor

🎉 New feature

Summary

Add an option to ign_install_all_headers so that a header is installed, but not included in the master header.

I need this feature because I introduced a bunch of warnings to everyone who includes msgs.hh when I deprecated SuppressWarning.hh in

Test it

TODO

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina requested a review from mjcarroll April 14, 2022 01:31
@github-actions github-actions bot added Gazebo 1️1️ Dependency of Gazebo classic version 11 🌱 garden Ignition Garden 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel labels Apr 14, 2022
# [EXCLUDE_FILES <excluded_headers>]
# [EXCLUDE_DIRS <dirs>]
# [GENERATED_HEADERS <headers>]
# [INSTALL_BUT_DONT_INCLUDE <headers>]
Copy link
Member

Choose a reason for hiding this comment

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

this parameter name is confusing to me

an alternative approach is to use EXCLUDE_FILES and then manually install the desired headers, as we don in sdformat:

it could benefit from an easier install helper so it doesn't need to pass ${IGN_INCLUDE_INSTALL_DIR_FULL}/${PROJECT_INCLUDE_DIR} as the destination

ign_install_includes will provide the first half of that path, but you'd still have to pass ${PROJECT_INCLUDE_DIR}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh much better, thanks!

@chapulina chapulina closed this Apr 14, 2022
@chapulina chapulina deleted the chapulina/2/install_include branch April 14, 2022 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏰 citadel Ignition Citadel 🏯 fortress Ignition Fortress 🌱 garden Ignition Garden Gazebo 1️1️ Dependency of Gazebo classic version 11

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants