Skip to content

[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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ferdymercury
Copy link
Collaborator

@ferdymercury ferdymercury commented Jun 19, 2025

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.

@ferdymercury
Copy link
Collaborator Author

@pcanal I see a fatal error: module file '/github/home/ROOT-CI/build/lib/MultiProc.pcm' is out of date and needs to be rebuilt: module file out of date in https://github.com/root-project/root/actions/runs/15753057412/job/44402174612?pr=19092

Copy link

github-actions bot commented Jun 19, 2025

Test Results

    18 files      18 suites   3d 5h 28m 55s ⏱️
 3 007 tests  3 004 ✅ 0 💤  3 ❌
52 855 runs  52 832 ✅ 0 💤 23 ❌

For more details on these failures, see this check.

Results for commit 835021f.

♻️ This comment has been updated with latest results.

@ferdymercury ferdymercury requested review from linev and guitargeek June 19, 2025 12:23
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.

2 participants