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 switch case fall through warning #345

Merged
merged 1 commit into from
Apr 18, 2022
Merged

fix switch case fall through warning #345

merged 1 commit into from
Apr 18, 2022

Conversation

ijnek
Copy link
Contributor

@ijnek ijnek commented Apr 17, 2022

🦟 Bug fix

Bug

Warning is raised when compiling on Ubuntu Jammy using g++11.

/home/ijnek/ignition/garden/src/ign-common/graphics/src/SVGLoader.cc:584:18: warning: this statement may fall through [-Wimplicit-fallthrough=]
  584 |         while (i < count)
      |                ~~^~~~~~~
/home/ijnek/ignition/garden/src/ign-common/graphics/src/SVGLoader.cc:602:7: note: here
  602 |       case 'Z':
      |       ^~~~
---

Summary

Simply add a break to the switch case statement as the fallthrough seems unintended.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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.

@ijnek ijnek requested a review from mjcarroll as a code owner April 17, 2022 08:47
@ijnek ijnek changed the title Ijnek switch case fall through fix switch case fall through warning Apr 17, 2022
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Apr 17, 2022
@ijnek ijnek changed the base branch from ign-common4 to main April 17, 2022 08:48
@ijnek
Copy link
Contributor Author

ijnek commented Apr 17, 2022

I'm going to guess DCO is failing because it didn't re-run the DCO test when I changed the PR base branch from ign-common4 to main, going to push a commit message change to see if that's the issue.

Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
@codecov
Copy link

codecov bot commented Apr 18, 2022

Codecov Report

Merging #345 (8e8de94) into main (153ce13) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #345   +/-   ##
=======================================
  Coverage   91.66%   91.66%           
=======================================
  Files           1        1           
  Lines          48       48           
=======================================
  Hits           44       44           
  Misses          4        4           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 153ce13...8e8de94. Read the comment docs.

@mjcarroll mjcarroll merged commit 1b1ad37 into gazebosim:main Apr 18, 2022
@ijnek ijnek deleted the ijnek-switch-case-fall-through branch April 18, 2022 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants