-
-
Notifications
You must be signed in to change notification settings - Fork 152
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: Drop support for Python 3.6 #329
Conversation
3a69cce
to
101ca02
Compare
101ca02
to
e74c23d
Compare
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.
Thanks for doing all this, and sorry you had to do all the work—I was going to take care of it shortly myself due to QtPy 2.1.0's release evidently approaching, but you got to it first.
As for the CI issue, it appears the qdialog tests need to be skipped when not using conda as well since the same thing happens (CI stalls), though you could try increasing the timeout on QtCore.QTimer.singleShot(100, dialog.accept)
since it could be the dialog just isn't ready in time.
I had a few comments here, and there were a couple other spots elsewhere that I submitted a PR against your branch, dalthviz#1 , to fix.
Also, you confirm that the modified skips can't simply be removed, right? Otherwise we get a lot of accumulating techdebt of skips over time.
PR: Update a couple additional instances of Python 3.6-specific code and comments
* Improve skip `test_patched_qheaderview` reason string * Remove unnecessary config for CI jobs Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
f7a3e78
to
ad51d68
Compare
Skip PySide6 on macOS with Python 3.7 Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
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.
LGTM now, thanks @dalthviz !
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!
Fixes #297