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

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

Merged
merged 50 commits into from
Aug 14, 2024

Conversation

nohlson
Copy link
Contributor

@nohlson nohlson commented Aug 6, 2024

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.

nohlson and others added 30 commits July 23, 2024 03:14
@nohlson
Copy link
Contributor Author

nohlson commented Aug 12, 2024

I have made the requested changes; please review again

@bedevere-app
Copy link

bedevere-app bot commented Aug 12, 2024

Thanks for making the requested changes!

: please review the changes made to this pull request.

Copy link
Contributor

@sethmlarson sethmlarson left a comment

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! :)

Copy link
Contributor

@sethmlarson sethmlarson left a comment

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 🚀

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

nohlson and others added 2 commits August 14, 2024 10:54
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Copy link
Member

@hugovk hugovk 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! 🚀

@hugovk hugovk merged commit 1cf624b into python:main Aug 14, 2024
36 checks passed
blhsing pushed a commit to blhsing/cpython that referenced this pull request Aug 22, 2024
…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 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