-
Notifications
You must be signed in to change notification settings - Fork 26
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
Remove support for PySide and PyQt4 from CI #285
Conversation
"pyqt": {"pyqt<4.12"}, # FIXME: build of 4.12-1 appears to be bad | ||
# XXX once pyqt5 is available in EDM, we will want it here | ||
"pyqt5": set(), | ||
"pyqt5": {"pyqt5"}, |
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.
This mapping will be inconsistent with traitsui and pyface, where the mapping is "pyqt": {"pyqt5"}
instead. It might add to the cognitive load for developers working across multiple ETS projects if envisage chooses a different naming.
I too preferred "pyqt5" in the toolkit name, but I deemed that opinion subjective. Afterall, this is only affecting developers' tools so this is easy to change one way or another.
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.
Yes, that's a good point. I'll change it. But in that case, for consistency, we should probably be using pyside
instead of pyside2
. :-)
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.
Actually, I think there is a good non-subjective reason to prefer pyqt5
over pyqt
, which is that it matches the names of the corresponding EDM and PyPI packages.
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.
I'm going to stick with the existing pyqt5
, both because it matches the previous behaviour on this repo (I'd rather not switch pyqt
from being valid and meaning one thing to still being valid and meaning something different) and because it matches the PyPI and EDM names.
Update the CI builds and etstool.py
xref: https://github.com/enthought/pyface/pull/512/files