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 trivial warning on 24.04 for JointAxis_TEST.cc #1402

Merged
merged 6 commits into from
Apr 22, 2024

Conversation

j-rivero
Copy link
Contributor

🦟 Bug fix

Summary

A build of the sdf14 on the upcoming Ubuntu Noble return 3 warnings on the JointAxis_TEST.cc:

‘multiplier’ may be used uninitialized

Adding a default value when constructing seems enough.

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

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Apr 16, 2024
Comment on lines 299 to 301
double multiplier = 0.0;
multiplier = mimicElem->Get<double>(
errors, "multiplier", multiplier).first;
Copy link
Member

@scpeters scpeters Apr 16, 2024

Choose a reason for hiding this comment

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

Suggested change
double multiplier = 0.0;
multiplier = mimicElem->Get<double>(
errors, "multiplier", multiplier).first;
double multiplier;
multiplier = mimicElem->Get<double>(
errors, "multiplier", 0.0).first;

This third argument is the default value; I think it's clearer to put the constant here instead of initializing multiplier.

Copy link
Member

Choose a reason for hiding this comment

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

looking at the rest of the file, I see that your proposed change matches the pattern in the rest of the file, so you can ignore this suggestion if you want to maintain consistency, and I'll fix the rest of the file in a separate pull request

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looking at the rest of the file, I see that your proposed change matches the pattern in the rest of the file, so you can ignore this suggestion if you want to maintain consistency, and I'll fix the rest of the file in a separate pull request

Looks good to me to have the changes under this PR 03cdb10 . Let me know if I'm missing any.

Copy link
Member

Choose a reason for hiding this comment

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

I just updated two string default values to "" in c2f3204. Everything else looks good; thanks!

src/JointAxis_TEST.cc Outdated Show resolved Hide resolved
src/JointAxis_TEST.cc Outdated Show resolved Hide resolved
j-rivero and others added 4 commits April 22, 2024 17:10
Co-authored-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@j-rivero j-rivero merged commit 2f2d8e1 into sdf14 Apr 22, 2024
11 checks passed
@j-rivero j-rivero deleted the jrivero/sdf14_warnings_noble branch April 22, 2024 17:32
aagrawal05 pushed a commit to aagrawal05/sdformat that referenced this pull request Aug 16, 2024
* Fix trivial warning on 24.04 for JointAxis_TEST.cc

---------

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Co-authored-by: Steve Peters <scpeters@openrobotics.org>
aagrawal05 pushed a commit to aagrawal05/sdformat that referenced this pull request Aug 16, 2024
* Fix trivial warning on 24.04 for JointAxis_TEST.cc

---------

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Co-authored-by: Steve Peters <scpeters@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants