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

Porting bitmessageqt to Qt5 #1389

Draft
wants to merge 34 commits into
base: v0.6
Choose a base branch
from
Draft

Porting bitmessageqt to Qt5 #1389

wants to merge 34 commits into from

Conversation

g1itch
Copy link
Collaborator

@g1itch g1itch commented Nov 5, 2018

Hello!

I realized that column width problem in QTableWidget was actually the Qt bug.

I mostly finished work in this branch though it still needs some testing.

@PeterSurda
Copy link
Member

Can we have some sort of fallback qtpy bundled?

@PeterSurda
Copy link
Member

PeterSurda commented Nov 9, 2018

The source doesn't look big, most of it are tests. And it's under MIT license.

@g1itch
Copy link
Collaborator Author

g1itch commented Nov 9, 2018

Probably yes. I thought of it, just forgot. I think it should be slim compatibility layer which just imports PyQt5.

BTW, I decided to finish UI refactoring in separate branch.

src/depends.py Outdated
passed = True
if QtCore.PYQT_VERSION < 0x40800:
if qtpy.PYQT_VERSION < '4.8':
Copy link
Member

Choose a reason for hiding this comment

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

These tests are wrong as they compare strings and it thinks '4.11.4' < '4.8'. You need to use

from distiutils.version import LooseVersion

and then wrap these comparisons in LooseVersion like qtpy does it.

src/depends.py Outdated
logger.error(
'This version of PyQt is too old. PyBitmessage requries'
' PyQt 4.8 or later.')
passed = False
if QtCore.QT_VERSION < 0x40700:
if qtpy.QT_VERSION < '4.7':
Copy link
Member

Choose a reason for hiding this comment

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

same problem here, should use LooseVersion

@PeterSurda PeterSurda self-requested a review November 11, 2018 17:26
@PeterSurda
Copy link
Member

I tried it, and it runs both with pyqt4 and pyqt5 and you can use the environment variable QT_API to override. Well done. I haven't looked at it in detail, but it seems pretty straightforward, just a lot of string replacements of Qt4 to QtPy. Please clean up the linter complaints and I'll try to do a full review asap.

@hub229ox
Copy link

qt4 is being less and less supported by the main distros

@g1itch
Copy link
Collaborator Author

g1itch commented Feb 12, 2019

Perhaps now you can do a review.

resolved pylint redefined-variable-type warnings,
marked autogenerated modules for skipping by pylint and flake8
on tabs "Send", "Blacklist" and "Network Status":
in qt5 it's probably true by default.
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.

7 participants