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

Add gazebosim/sdformat and its dependencies to BCR #3172

Merged

Conversation

udaya2899
Copy link
Contributor

To be reviewed by @shameekganguly or @mjcarroll. The version name I chose is a <date>.1 format since this doesn't seem to be a stable release.

@bazel-io
Copy link
Member

Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (gz-math, gz-utils, sdformat) have been updated in this PR. Please review the changes.

@udaya2899
Copy link
Contributor Author

While testing this locally,

In file included from external/sdformat~20241112.1/src/EmbeddedSdf.hh:24:
external/sdformat~20241112.1/include/sdf/Types.hh:28:10: error: module sdformat~20241112.1//:sdformat does not depend on a module exporting 'gz/utils/NeverDestroyed.hh'
   28 | #include <gz/utils/NeverDestroyed.hh>
      |          ^
1 error generated.
Target @@sdformat~20241112.1//:sdformat failed to build

I think we need to add @gz-utils//:NeverDestroyed to the @sdformat//:sdformat deps in the BUILD file.

@udaya2899
Copy link
Contributor Author

@bazel-io skip_check unstable_url

1 similar comment
@meteorcloudy
Copy link
Member

@bazel-io skip_check unstable_url

@bazel-io bazel-io added the skip-url-stability-check Skip the URL stability check for the PR label Nov 13, 2024
Copy link

@shameekganguly shameekganguly left a comment

Choose a reason for hiding this comment

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

Thanks Udaya! I suggested a few changes for the presubmit setups. Other than that, we should wait for your upstream PRs in gz-math and sdformat to be submitted, make releases for all of those packages and then request this PR to be merged in.

modules/gz-math/20241112.1/presubmit.yml Outdated Show resolved Hide resolved
modules/gz-math/20241112.1/presubmit.yml Outdated Show resolved Hide resolved
@meteorcloudy meteorcloudy added the presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval label Nov 15, 2024
@udaya2899
Copy link
Contributor Author

Ready for review again, presubmits pass: @shameekganguly or @mjcarroll for a final pass. @meteorcloudy for merging

@Wyverald Wyverald merged commit 35f4e80 into bazelbuild:main Nov 18, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval skip-url-stability-check Skip the URL stability check for the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants