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.

Sign up for GitHub

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

Jump to bottom

SG-30560 Code Cleanup: Removing Qt4/PySide/PyQt4 compatibility in Toolkit #973

Merged
merged 13 commits into from
Jun 20, 2024
Merged

SG-30560 Code Cleanup: Removing Qt4/PySide/PyQt4 compatibility in Toolkit #973

merged 13 commits into from
Jun 20, 2024

Conversation

eduardoChaucaGallegos
Copy link
Contributor

We are dropping the support and compatibility with Qt4. So we are free to delete all the code in Toolkit/Desktop which supports PySide and PyQt4.

@codecov Codecov
Copy link

codecov bot commented May 16, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 59.09091% with 18 lines in your changes missing coverage. Please review.

Project coverage is 79.89%. Comparing base (0edcee4) to head (6ada45c).

Files Patch % Lines
...nk/authentication/sso_saml2/core/sso_saml2_core.py 58.13% 18 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #973      +/-   ##
==========================================
+ Coverage   79.43%   79.89%   +0.45%     
==========================================
  Files         199      198       -1     
  Lines       20870    20719     -151     
==========================================
- Hits        16579    16554      -25     
+ Misses       4291     4165     -126

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

julien-lang
julien-lang reviewed May 16, 2024
View reviewed changes
Copy link
Contributor

@julien-lang julien-lang left a comment

Choose a reason for hiding this comment

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

A good start. I love when we remove code :) Thanks @eduardoChaucaGallegos

python/tank/authentication/ui/qt5_like_line_edit.py Outdated Show resolved Hide resolved
python/tank/util/qt_importer.py Outdated Show resolved Hide resolved
tests/util_tests/test_qt_importer.py Outdated Show resolved Hide resolved
carlos-villavicencio-adsk
carlos-villavicencio-adsk reviewed May 16, 2024
View reviewed changes
Copy link
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk left a comment

Choose a reason for hiding this comment

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

Looks good so far. I just pointed out some comments here and there.

python/tank/platform/engine.py Show resolved Hide resolved
python/tank/util/qt_importer.py Show resolved Hide resolved
python/tank/util/qt_importer.py Outdated Show resolved Hide resolved
tests/util_tests/test_qt_importer.py Show resolved Hide resolved
@eduardoChaucaGallegos eduardoChaucaGallegos marked this pull request as ready for review May 23, 2024 15:54
julien-lang
julien-lang approved these changes May 30, 2024
View reviewed changes
Copy link
Contributor

@julien-lang julien-lang left a comment

Choose a reason for hiding this comment

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

LGTM!

carlos-villavicencio-adsk
carlos-villavicencio-adsk approved these changes May 30, 2024
View reviewed changes
Copy link
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk left a comment

Choose a reason for hiding this comment

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

Minimal suggestion. All good on my side.

tests/util_tests/test_qt_importer.py Outdated Show resolved Hide resolved
@julien-lang julien-lang mentioned this pull request Jun 4, 2024
Merged
julien-lang
julien-lang requested changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@julien-lang julien-lang left a comment

Choose a reason for hiding this comment

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

Only one small thing to change.

python/tank/platform/engine.py Outdated Show resolved Hide resolved
carlos-villavicencio-adsk
carlos-villavicencio-adsk approved these changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk left a comment

Choose a reason for hiding this comment

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

LGTM

julien-lang
julien-lang approved these changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@julien-lang julien-lang left a comment

Choose a reason for hiding this comment

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

LGTM!

@eduardoChaucaGallegos eduardoChaucaGallegos merged commit 7c52d07 into master Jun 20, 2024
27 of 28 checks passed
@eduardoChaucaGallegos eduardoChaucaGallegos deleted the ticket/SG-30560-code-cleanup-qt4 branch June 20, 2024 16:46
@julien-lang julien-lang mentioned this pull request Jun 21, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlos-villavicencio-adsk carlos-villavicencio-adsk carlos-villavicencio-adsk approved these changes

@julien-lang julien-lang julien-lang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eduardoChaucaGallegos @julien-lang @carlos-villavicencio-adsk