-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
PR: Ensure QMenu
and QToolBar
isinstance
checks succeed and remove unneeded wrapper classes
#507
Conversation
for more information, see https://pre-commit.ci
QMenu
submenus are QMenu
instancesQMenu
, QToolBar
and other classes isinstances
checks succeed
2309c82
to
27de0f4
Compare
QMenu
, QToolBar
and other classes isinstances
checks succeedQMenu
, QToolBar
and other classes isinstance
checks succeed
QMenu
, QToolBar
and other classes isinstance
checks succeedQMenu
, QToolBar
and other classes isinstance
checks succeed and remove unneeded wrapper classes
QMenu
, QToolBar
and other classes isinstance
checks succeed and remove unneeded wrapper classes QMenu
and QToolBar
isinstance
checks succeed and remove unneeded wrapper classes
My bad. I haven't thought of such a consequence. The changes you've proposed are the only way, then. Apparently, I've removed the test case that led me to the inheritance of the classes. So, I can't review it to see if it was erroneous. As the tests pass, it should be OK. |
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.
Looks good to me, thanks @dalthviz!
@dalthviz, given the severity of this problem, I think we should release 2.4.3 asap. What do you think? |
Fixes #502
Fixes #505