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

Use ignition::gazebo:: in class instantiation #1967

Merged
merged 2 commits into from
May 2, 2023

Conversation

scpeters
Copy link
Member

🦟 Bug fix

Fixes macOS build.

Summary

The macOS build has been broken since #1646:

In other packages, I have avoided using the gz::sim namespace when implementing classes, so doing it here seems to fix the macOS build. The ABI build is broken too, but that needs a different fix.

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.

Fixes macOS build.

Signed-off-by: Steve Peters <[email protected]>
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Apr 21, 2023
@codecov
Copy link

codecov bot commented Apr 21, 2023

Codecov Report

Merging #1967 (d3484b0) into ign-gazebo3 (875de1b) will increase coverage by 0.07%.
The diff coverage is n/a.

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

@@               Coverage Diff               @@
##           ign-gazebo3    #1967      +/-   ##
===============================================
+ Coverage        77.96%   78.04%   +0.07%     
===============================================
  Files              255      255              
  Lines            15080    15080              
===============================================
+ Hits             11757    11769      +12     
+ Misses            3323     3311      -12     
Impacted Files Coverage Δ
src/EntityComponentManager.cc 89.09% <ø> (ø)
src/Link.cc 93.54% <ø> (ø)
src/Model.cc 95.83% <ø> (ø)
src/SdfEntityCreator.cc 90.44% <ø> (ø)
src/ServerConfig.cc 90.28% <ø> (ø)
src/SystemLoader.cc 81.96% <ø> (ø)
src/TestFixture.cc 100.00% <ø> (ø)
src/World.cc 96.87% <ø> (ø)
...rc/systems/ackermann_steering/AckermannSteering.cc 85.54% <ø> (ø)
src/systems/air_pressure/AirPressure.cc 73.17% <ø> (ø)
... and 30 more

... and 1 file with indirect coverage changes

@mjcarroll
Copy link
Contributor

macOS build doesn't seem to like that.

@scpeters
Copy link
Member Author

macOS build doesn't seem to like that.

it's because I removed broken bottles in osrf/homebrew-simulation#2242, which maybe I didn't have to do as a separate step? At any rate, rebuilding the gz-common3 bottle in osrf/homebrew-simulation#2245 which should fix it

@azeey azeey mentioned this pull request Apr 21, 2023
8 tasks
@scpeters
Copy link
Member Author

macOS build doesn't seem to like that.

it's because I removed broken bottles in osrf/homebrew-simulation#2242, which maybe I didn't have to do as a separate step? At any rate, rebuilding the gz-common3 bottle in osrf/homebrew-simulation#2245 which should fix it

homebrew build is now unstable, I believe because it ran on macos-monterey and there are some failing tests due to gazebosim/gz-physics#442

@scpeters
Copy link
Member Author

I believe the ABI checker is failing due to #1971

@scpeters
Copy link
Member Author

scpeters commented May 2, 2023

CI is 🟢 ! Hooray!

@scpeters scpeters merged commit a2a2c85 into ign-gazebo3 May 2, 2023
@scpeters scpeters deleted the scpeters/fix_macos_3 branch May 2, 2023 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants