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

Windows: Remove unused Qt5 block and reference #10697

Merged

Conversation

Dentomologist
Copy link
Contributor

Remove a Windows/Qt5 only section of Main.cpp now that Windows has a minimum Qt version of 6.3.

The SetCurrentUserStyle comment was overly specific; the bug also occurs with light-mode macOS, Windows with Qt 6.3, or an empty stylesheet_name.

@MayImilae
Copy link
Contributor

MayImilae commented May 26, 2022

macOS still hasn't been updated to Qt6 yet. Is this PR fine with that? (I'd be very surprised if it wasn't based on the code but asking anyway)

@Dentomologist
Copy link
Contributor Author

Yep, that code block effectively didn't exist for any platform besides Windows.

@AdmiralCurtiss
Copy link
Contributor

IMO we should wait on removing this simple kind of glue until Qt5 doesn't work anymore for other reasons -- right now it should still compile and run fine, after all. These few lines don't hurt anything by being there.

@Dentomologist
Copy link
Contributor Author

Nothing's going to break if we leave it alone, but having this code block which appears at a glance to do something but actually doesn't is a tiny bit of technical debt.

I haven't seen any discussion on how long we'll maintain Qt5 support on other platforms, but it wouldn't surprise me if it lasted a year or two. Is there some benefit to keeping this around until then instead of removing it now?

@Tilka Tilka merged commit 57e444c into dolphin-emu:master May 28, 2022
10 checks passed
@Dentomologist Dentomologist deleted the windows_remove_qt5_code_and_references branch May 30, 2022 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants