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

Unknown compiler options for clang shouldn't be show-stoppers #2967

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented Aug 12, 2024

Description

This is a workaround for a CI error that has been cropping up once in awhile recently:

Error while processing /home/runner/work/moveit2/moveit2/.work/target_ws/src/moveit2/moveit_ros/moveit_servo/src/utils/common.cpp.
error: unknown warning option '-Wno-maybe-uninitialized'; did you mean '-Wno-uninitialized'? [clang-diagnostic-unknown-warning-option]

From https://github.com/moveit/moveit2/actions/runs/10335417126/job/28609933300 for one example

I don't think there's actually a potentially uninitialized variable there, at least not a new one in this PR. I think what happened is, clang-tidy was cached previously for this cpp file. That PR happened to touch that cpp file, which caused clang-tidy to re-run for it. And something about clang-tidy has changed since the last time it ran.

Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

I additionally searched for this issue and found this relevant post: https://stackoverflow.com/a/41673702

So this change makes sense to me, but would be nice for someone with more software chops than me to give it the final approval.

Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

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

Maybe add a comment to remove that option in the future if clang-tidy is fixed.

@AndyZe AndyZe force-pushed the clang_no_unknown_warnings branch from 1fdba8d to 805c094 Compare August 12, 2024 13:35
@AndyZe AndyZe enabled auto-merge August 12, 2024 13:36
@AndyZe AndyZe force-pushed the clang_no_unknown_warnings branch from 805c094 to d5a5d52 Compare August 12, 2024 16:56
@AndyZe AndyZe force-pushed the clang_no_unknown_warnings branch from d5a5d52 to 3bccace Compare August 12, 2024 17:22
@AndyZe AndyZe requested review from sea-bass and rhaschke August 12, 2024 18:32
@AndyZe AndyZe added this pull request to the merge queue Aug 12, 2024
Merged via the queue into moveit:main with commit bb69934 Aug 12, 2024
10 of 12 checks passed
@AndyZe AndyZe deleted the clang_no_unknown_warnings branch August 12, 2024 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants