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

Fix Windows and Doxygen #1643

Merged
merged 3 commits into from
Aug 17, 2022
Merged

Fix Windows and Doxygen #1643

merged 3 commits into from
Aug 17, 2022

Conversation

chapulina
Copy link
Contributor

🦟 Bug fix

Summary

Windows

The Windows build has been broken since

This fixes in adding IGNITION_GAZEBO_GUI_VISIBLE. The GZ_SIM equivalent has already been added to gz-sim7 in #1633.

Doxygen

After a lot of digging, it turned out that the Doxygen crashes described in #1409 were caused by the inline namespace. The Doxygen version on Jammy (1.9.1) can't handle those well.

Warnings

Also fixed some compiler warnings that I saw on Jammy.

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.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina added bug Something isn't working documentation Improvements or additions to documentation labels Aug 11, 2022
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Aug 11, 2022
@chapulina chapulina added the Windows Windows support label Aug 11, 2022
@codecov
Copy link

codecov bot commented Aug 11, 2022

Codecov Report

Merging #1643 (55b554d) into ign-gazebo6 (8c9489d) will increase coverage by 0.02%.
The diff coverage is 0.00%.

@@               Coverage Diff               @@
##           ign-gazebo6    #1643      +/-   ##
===============================================
+ Coverage        64.40%   64.43%   +0.02%     
===============================================
  Files              320      320              
  Lines            25892    25892              
===============================================
+ Hits             16677    16683       +6     
+ Misses            9215     9209       -6     
Impacted Files Coverage Δ
src/LevelManager.cc 89.19% <ø> (ø)
src/systems/elevator/Elevator.hh 100.00% <ø> (ø)
src/systems/elevator/ElevatorCommonPrivate.hh 0.00% <ø> (ø)
src/systems/elevator/ElevatorStateMachine.hh 0.00% <ø> (ø)
src/systems/elevator/state_machine/ActionsImpl.hh 92.30% <ø> (ø)
...elevator/state_machine/ElevatorStateMachineImpl.hh 73.52% <ø> (ø)
src/systems/elevator/state_machine/EventsImpl.hh 100.00% <ø> (ø)
src/systems/elevator/state_machine/GuardsImpl.hh 100.00% <ø> (ø)
src/systems/elevator/state_machine/StatesImpl.hh 94.73% <ø> (ø)
src/systems/elevator/utils/DoorTimer.hh 0.00% <ø> (ø)
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@chapulina chapulina requested a review from Blast545 August 11, 2022 21:34
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
@Blast545
Copy link
Contributor

jammy builds are failing on UNIT_Gui_TEST and I haven't seen this one before, could it be related to the changes made on: src/gui/QuickStartHandler.hh?

@Blast545
Copy link
Contributor

@ros-pull-request-builder retest this please

@chapulina chapulina requested a review from mjcarroll August 17, 2022 21:27
@Blast545
Copy link
Contributor

I wanted to take a closer look to the errors on Jammy and UNIT_Gui_TEST before approving, I'll track this internally with @Crola1702

@chapulina
Copy link
Contributor Author

I wanted to take a closer look to the errors on Jammy and UNIT_Gui_TEST before approving, I'll track this internally with @Crola1702

Thanks! I believe those are pre-existing. It's good to get this PR in quickly so Windows CI runs and it doesn't get more broken.

@chapulina chapulina merged commit 4f93cde into ign-gazebo6 Aug 17, 2022
@chapulina chapulina deleted the chapulina/6/win_fix branch August 17, 2022 21:37
chapulina added a commit that referenced this pull request Aug 29, 2022
Signed-off-by: Louise Poubel <[email protected]>
chapulina added a commit that referenced this pull request Aug 30, 2022
Signed-off-by: Louise Poubel <[email protected]>
@j-rivero j-rivero mentioned this pull request Sep 16, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation 🏯 fortress Ignition Fortress Windows Windows support
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Doxygen segfaults on ign-gazebo6 and ign-gazebo7
2 participants