Skip to content

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

Merged
dalthviz merged 9 commits intospyder-ide:masterfrom
StSav012:make_addAction_compatible_with_Qt6
Aug 14, 2023
Merged

PR: Make QMenu.addAction and QToolBar.addAction compatible with Qt6 arguments' order#437
dalthviz merged 9 commits intospyder-ide:masterfrom
StSav012:make_addAction_compatible_with_Qt6

Conversation

@StSav012
Copy link
Copy Markdown
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
Copy Markdown
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 👍

@StSav012
Copy link
Copy Markdown
Contributor Author

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

@dalthviz
Copy link
Copy Markdown
Member

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

@dalthviz
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
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