PR: Make warning usage consistant and refine messages#398
Merged
dalthviz merged 1 commit intospyder-ide:masterfrom Jan 17, 2023
CAM-Gerlach:refine-warning-hierarchy
Merged
PR: Make warning usage consistant and refine messages#398dalthviz merged 1 commit intospyder-ide:masterfrom CAM-Gerlach:refine-warning-hierarchy
dalthviz merged 1 commit intospyder-ide:masterfrom
CAM-Gerlach:refine-warning-hierarchy
Conversation
Member
Author
|
(Oops, fell asleep before I went back and hit the ready for review button) |
ccordoba12
reviewed
Jan 14, 2023
Member
ccordoba12
left a comment
There was a problem hiding this comment.
Two small suggestions for you @CAM-Gerlach, otherwise looks good to me.
ccordoba12
approved these changes
Jan 17, 2023
Member
ccordoba12
left a comment
There was a problem hiding this comment.
Looks good to me now, thanks @CAM-Gerlach!
Member
|
Also @CAM-Gerlach should this close #395 ? |
Member
Author
|
It may help with it some, but it isn't clear that it would resolve the actual issue the user experienced, since from the very limited information provided it is not at all clear what that actually is. I suggest we wait at least a couple weeks to see if they respond. Also, you mentioned documenting the error/warning hierarchy somewhere, which is a good idea to do at some point. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, as noted in issue #395 , a generic
RuntimeWarningis emitted when falling back to a non-selected binding, instead of QtPy's ownPythonQtWarningsubclass. Furthermore, the text could perhaps be clarified a bit. I also noticed a few other issues when making this changed, which I fixed.Therefore, this PR:
PythonQtWarninginstead of a genericRuntimeWarningwhen falling back to an unselected binding, consistent with the other warnings and errors that use QtPy-specific subclassesPythonQtWarninginherit fromRuntimeWarninginstead of a genericWarning, to be consistent with all the other errors and warnings and with whatRuntimeWarningis meant for, and ensure the previous replacement is fully backwards compatiblePythonQtWarningandPythonQtError!rinstead of hardcoded quotes