Skip to content
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

Migrate some IGNITION_* macros #62

Merged
merged 4 commits into from
May 18, 2022
Merged

Conversation

scpeters
Copy link
Member

🦟 Bug fix

Part of gazebo-tooling/release-tools#698

Summary

The IGNITION_* macros from the autogenerated Export.hh have been migrated to support GZ_* as well as of gazebosim/gz-cmake#249, so the GZ_UTILS_VISIBLE macro is now being used (97a1cfa). This also tick-tocks most of the macros in config.hh.in (notably not the *_BUILD_TYPE_* macros, which are problematic) and migrates to GZ_UTILS_VERSION_NAMESPACE where relevant.

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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.

scpeters added 2 commits May 16, 2022 17:08
Signed-off-by: Steve Peters <[email protected]>
The most used is GZ_UTILS_VERSION_NAMESPACE.

Signed-off-by: Steve Peters <[email protected]>
@scpeters scpeters requested a review from azeey as a code owner May 17, 2022 01:17
@github-actions github-actions bot added the 🌱 garden Ignition Garden label May 17, 2022
@chapulina chapulina added the ign to gz Renaming Ignition to Gazebo. label May 17, 2022
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

LGTM, I guess there's no way to trigger a warning when the IGNITION_ versions are used, right? It's more of a slient-tick and then tock, but I guess that's what we can do.

I believe this is backportable to Citadel and Fortress if we want to reduce the diff between the branches. I don't think we should do that now, we may have time for it after feature freeze.

include/gz/utils/config.hh.in Show resolved Hide resolved
@scpeters
Copy link
Member Author

LGTM, I guess there's no way to trigger a warning when the IGNITION_ versions are used, right? It's more of a slient-tick and then tock, but I guess that's what we can do.

once you mentioned that, it occurred to me that we could move the IGNITION_* macros to ignition/utils/config.hh, so I did so in 06f7b94

how does that look?

@chapulina chapulina merged commit 521864c into main May 18, 2022
@chapulina chapulina deleted the scpeters/migrate_some_macros branch May 18, 2022 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden ign to gz Renaming Ignition to Gazebo.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants