-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
fix #10342: put location into warning exceptions #10343
fix #10342: put location into warning exceptions #10343
Conversation
as the warning systems own warn_explicit looses the information we add them explicitly to the warning exceptions
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.
I suppose this also needs a changelog, as improvement
?
It's unreleased, so I consider it part of the initial change |
Fair enough, did not realize that. 👍 |
@The-Compiler i'd like to get your input before merging |
I wanted to try this by installing
Is your branch outdated or something, or is that just a setuptools-scm thing? |
@The-Compiler i believe that i didn't push the tags to my repo, let me fix |
@The-Compiler updated |
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.
Great idea with only adding it if warnings are exceptions! Only a small suggestion, but looks good to go.
Co-authored-by: Florian Bruhin <me@the-compiler.org>
as the warning systems own warn_explicit looses the information we add them explicitly to the warning exceptions
@The-Compiler this should resolve the issue you noted
closes #10342