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

Retry fix for unsafe initialization in graph classes #1460

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

scpeters
Copy link
Member

🦟 Bug fix

Follow-up to gazebosim/gz-math#612.

Summary

The is a second attempt at following up on the fix for potentially unsafe initialization issues in the graph::Edge and graph::Vertex classes after the first attempts in gazebosim/gz-math#606 and #1458 were reverted in gazebosim/gz-math#609 and #1459 due to test failures induced in gz-sim (documented in osrf/buildfarm-tools#67 (comment)). This pull request should be merged after gazebosim/gz-math#612, which uses a static local variable instead of dynamic memory allocation and doesn't cause gz-sim test failures in my testing.

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.

Follow-up to gz-math#606.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters scpeters requested a review from azeey as a code owner July 19, 2024 04:47
@github-actions github-actions bot added the 🏛️ ionic Gazebo Ionic label Jul 19, 2024
@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 29, 2024
@scpeters
Copy link
Member Author

I've merged gazebosim/gz-math#612 and rebuilt nightly debs, and CI is almost finished and all completed jobs have passed

@scpeters
Copy link
Member Author

scpeters commented Aug 1, 2024

this will fix the clang warnings on main:

@scpeters scpeters merged commit fd81923 into main Aug 1, 2024
11 checks passed
@scpeters scpeters deleted the scpeters/retry_graph_init branch August 1, 2024 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection 🏛️ ionic Gazebo Ionic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants