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

PR: Make QMenu.addAction and QToolBar.addAction compatible with Qt6 arguments' order #437

Merged

Conversation

StSav012
Copy link
Contributor

@StSav012 StSav012 commented Jul 5, 2023

Surprisingly, I've stumbled upon that in Qt5 and Qt6, addAction accepts arguments in different order. So, here's the monkey patch for that.

I've checked other derivatives of QWidget, and only QMenu, QToolBar, and QLineEdit have non-trivial addAction. I haven't patched the latter, for it hasn't changed between Qt5 and Qt6. So, this is the patch for QMenu and QToolBar.

I don't actually know how to use the receiver: QObject and the member: bytes arguments of addAction, so I haven't tested them.

@StSav012 StSav012 marked this pull request as ready for review July 5, 2023 12:10
@ccordoba12 ccordoba12 requested a review from dalthviz July 5, 2023 12:38
@ccordoba12 ccordoba12 added this to the v2.4.0 milestone Jul 5, 2023
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @StSav012 for your work here and sorry for the late review! Left some comments regarding the Qt version usage but other than that this LGTM 👍

qtpy/QtWidgets.py Outdated Show resolved Hide resolved
qtpy/QtWidgets.py Show resolved Hide resolved
@StSav012
Copy link
Contributor Author

The tests have failed, but that's not my fault.

@dalthviz
Copy link
Member

Yep, the failing errors in the CI are unrelated to this and should be fixed after we merge #443

@dalthviz
Copy link
Member

dalthviz commented Aug 14, 2023

Just merged #443 . Updating the branch again with the base branch should make the CI pass 👍

@StSav012
Copy link
Contributor Author

Yeah, that's more like it. I'm sorry I haven't looked through the other pending PRs.

The tests have indeed become much faster with mamba. Great job!

Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @StSav012 !

@dalthviz dalthviz merged commit 3ba97e3 into spyder-ide:master Aug 14, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants