-
Notifications
You must be signed in to change notification settings - Fork 95
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
Conversation
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
src/JointAxis_TEST.cc
Outdated
double multiplier = 0.0; | ||
multiplier = mimicElem->Get<double>( | ||
errors, "multiplier", multiplier).first; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
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>
* 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>
* 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>
🦟 Bug fix
Summary
A build of the
sdf14
on the upcoming Ubuntu Noble return 3 warnings on theJointAxis_TEST.cc
:Adding a default value when constructing seems enough.
Checklist
codecheck
passed (See contributing)