Skip to content

gh-112301: Add Warning Count to Warning Check Tooling - #122711

Merged
hugovk merged 50 commits into
python:mainfrom
nohlson:add-warning-count-to-warning-ignore-files
Aug 14, 2024
Merged

gh-112301: Add Warning Count to Warning Check Tooling#122711
hugovk merged 50 commits into
python:mainfrom
nohlson:add-warning-count-to-warning-ignore-files

Conversation

@nohlson

@nohlsonnohlson commented Aug 6, 2024•
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

This adds the feature of specifying how many warnings should be ignored in a particular file for the check compiler warning tooling. This makes adding a particular file to the warning ignore files a bit more tolerable because it means that changes that introduce more warnings to a file will be caught. If a warning is fixed by a change but a new warning is introduced this will not be caught, however. That would require maintaining a list of specific warnings to ignore, which would be a bit overkill.

This change also removes duplicate warnings from being processed by the tooling, so that the accurate number of warnings per file can be specified when ignoring warnings in the .warningignore_* file.

nohlsonand others added 30 commits July 23, 2024 03:14
@nohlson
Create simple warning check tool and add to ubuntu build and test job
5148727
@nohlson
Add flags to check warnings script to fail on regression or improvement
3bd5a10
@nohlson
Remove redundant comment
6813003
@nohlson
Rename warnigore file to warningignore
ab4d754
@nohlson
Use regex to extract json arrays
615d228
@nohlson
Trim whitespace
fc0a60b
@nohlson
Test on github unexpected improvement
7793d80
@nohlson
Add config for improve fail check
99715d2
@nohlson
Revert to prod check warning state
bc44ec2
@blurb-it@nohlson
📜🤖 Added by blurb_it.
e2ca75f
@nohlson
Refactor creating set of files with warnings to a dedicated function
bd1634e
@nohlson
Move cflags configure option to top level build configuration
b07b1d6
@nohlson
Add json diagnostics to ubuntu configuration as first argument
e1954a5
@nohlson
Add newline to news
5935143
@nohlson
Create simple warning check tool and add to ubuntu build and test job
7f1a238
@nohlson
Add macos warning checks to GitHub actions
144136e
@nohlson
Revert reusable-macos.yml for environment variables
3dd40fd
@nohlson
Update paths
1158f53
@nohlson
Test unexpected improvement
cb51b4f
@nohlson
Remove warning ignore
77e0f6e
@nohlson
Add json output option to macos configure job
3e1d75f
@nohlson
Add common dictionary format when parsing warnings
b5cd58a
@nohlson
Remove configure option for macos job
02f313e
@nohlson
Print out json version of compiler output
83d1ed7
@nohlson
Remove old version of warning ignore file
08a6f6d
@nohlson
Remove compiler output print diagnostic
7be8ee6
@blurb-it
📜🤖 Added by blurb_it.
8b0a2ee
@nohlson
Remove superfluous comment
49cbd87
@nohlson
oMerge branch 'main' into add-macos-warnings-tracking-tooling
522f27c
@nohlson
Add period to news
b654a84
hugovk
hugovk reviewed Aug 7, 2024
View reviewed changes
Comment threadTools/build/check_warnings.py Outdated
Comment threadTools/build/check_warnings.py Outdated
Comment threadTools/build/check_warnings.py Outdated
@nohlson

nohlson commented Aug 12, 2024

Copy link
Copy Markdown
ContributorAuthor

I have made the requested changes; please review again

@bedevere-app

bedevere-appBot commented Aug 12, 2024

Copy link
Copy Markdown

Thanks for making the requested changes!

: please review the changes made to this pull request.

sethmlarson
sethmlarson reviewed Aug 13, 2024
View reviewed changes

@sethmlarsonsethmlarson left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Handful of review comments for you, otherwise looking good! :)

Comment threadTools/build/check_warnings.py Outdated
Comment threadTools/build/check_warnings.py Outdated
Comment threadTools/build/check_warnings.py
sethmlarson
sethmlarson approved these changes Aug 13, 2024
View reviewed changes

@sethmlarsonsethmlarson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks Nate! Really excited to see this land 🚀

hugovk
hugovk reviewed Aug 14, 2024
View reviewed changes

@hugovkhugovk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reminder: you can commit these changes via the GitHub UI: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request

Comment threadTools/build/check_warnings.py Outdated
Comment threadTools/build/check_warnings.py Outdated
nohlsonand others added 2 commits August 14, 2024 10:54
@nohlson@hugovk
Update Tools/build/check_warnings.py
ca519a3
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@nohlson@hugovk
Update Tools/build/check_warnings.py
f370442
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@nohlsonnohlson mentioned this pull request Aug 14, 2024
Merged
hugovk
hugovk approved these changes Aug 14, 2024
View reviewed changes

@hugovkhugovk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you! 🚀

@hugovk
hugovk merged commit 1cf624b into python:mainAug 14, 2024
blhsing pushed a commit to blhsing/cpython that referenced this pull request Aug 22, 2024
@nohlson@blurb-it
@hugovk@blhsing
pythongh-112301: Add warning count to warning check tooling (python#1…
065615d
…22711)
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovkhugovkhugovk approved these changes
@sethmlarsonsethmlarsonsethmlarson approved these changes
@ezio-melottiezio-melottiAwaiting requested review from ezio-melotti

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nohlson@hugovk@sethmlarson