-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[ci] Fail CI if missing override instead of just warning #19092
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
Conversation
@pcanal I see a |
Test Results 20 files 20 suites 3d 8h 43m 57s ⏱️ Results for commit 0c8d97e. ♻️ This comment has been updated with latest results. |
Yes, this is also my observation. |
Thanks @linev for the feedback. Btw, do you approve the changes in roottest/io ? |
For this part clearly Philippe @pcanal is responsible |
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.
Thank you!
Currently, many override warnings remain unnoticed since the gcc-problem-matcher of the GitHub CI does not work correctly, and it's also easy to forget those annotations if you do not visit the "Files changed" tab.
If some devs later use dev=ON and testing=ON, this leads to failures for compilers that auto-enable -Winconsistent-missing-override.
To prevent this, we enable the suggest-override warning in the build of debian with dev=ON instead of in the one from alma, this way we are sure of always failing if some does a PR that forgets an override, forcing pull-requesters to fix their issues before merging.
Within this PR: also fix missing overrides uncovered by turning ON this flag in the dev=ON build.