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.

Sign up for GitHub

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

Jump to bottom

Fix test sources with latest cppcheck #6726

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
Draft

Fix test sources with latest cppcheck #6726

wants to merge 19 commits into from

Conversation

MNikoliCC
Copy link
Contributor

@MNikoliCC MNikoliCC commented Dec 10, 2024 •
edited
Loading

Description
Fix cpp code to pass the test sources with latest cppcheck.

Tasks
Add the list of tasks of this PR.

  • Fix cpp code issues

@MNikoliCC MNikoliCC self-assigned this Dec 10, 2024
@MNikoliCC MNikoliCC requested a review from a team as a code owner December 10, 2024 10:03
@MNikoliCC MNikoliCC marked this pull request as draft December 10, 2024 10:03
@MNikoliCC MNikoliCC added test sources Start the sources test on all platforms optimization Make the code faster to execute labels Dec 10, 2024
@MNikoliCC MNikoliCC marked this pull request as ready for review December 10, 2024 15:06
lukicdarkoo
lukicdarkoo previously approved these changes Dec 10, 2024
View reviewed changes
Copy link
Member

@lukicdarkoo lukicdarkoo left a comment

Choose a reason for hiding this comment

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

Thank you!

omichel
omichel reviewed Dec 11, 2024
View reviewed changes
Copy link
Member

@omichel omichel left a comment

Choose a reason for hiding this comment

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

Just something looking strange. I am surprised the tests passed.

src/webots/nodes/WbRobot.cpp Outdated
@@ -270,7 +270,8 @@ void WbRobot::addDevices(WbNode *node) {
if (!node)
return;

if (node != this && dynamic_cast<const WbRobot *>(node))
const WbRobot *robotNode = dynamic_cast<const WbRobot *>(node);
if (node != this && robrobotNodeot)
Copy link
Member

Choose a reason for hiding this comment

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

That looks wrong (typo)

Copy link
Member

Choose a reason for hiding this comment

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

This a good catch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing this out

@lukicdarkoo lukicdarkoo added the test webots build Start the build tests label Dec 11, 2024
@omichel omichel added the test suite Start the test suite label Dec 11, 2024
@MNikoliCC
Copy link
Contributor Author

After reviewing the issue with latest cppcheck, I suggest not to make further changes at this time. Addressing the warnings would require adding const to a large number of methods, which involves verifying that each method does not modify any member variables of the passed object.

While it would be possible to add const indiscriminately to pass the checks, this approach may not be entirely safe or appropriate without thorough validation. Therefore, I'm opting to leave the current implementation as is.

@lukicdarkoo lukicdarkoo marked this pull request as draft December 12, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukicdarkoo lukicdarkoo lukicdarkoo left review comments

@omichel omichel omichel left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@MNikoliCC MNikoliCC

Labels
optimization Make the code faster to execute test sources Start the sources test on all platforms test suite Start the test suite test webots build Start the build tests
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MNikoliCC @omichel @lukicdarkoo