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

Disable update checks on versions before Windows 10 #19295

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

sledgehammer999
Copy link
Member

On versions before Windows 10 the user is running the Qt5 build.
The next qBittorrent release will change to Qt6 as the default build.
It is counterintuitive to tell the user about available updates when they are running an incompatible Windows version.

@sledgehammer999 sledgehammer999 added the OS: Windows Issues specific to Windows label Jul 9, 2023
@sledgehammer999 sledgehammer999 added this to the 4.5.5 milestone Jul 9, 2023
@sledgehammer999
Copy link
Member Author

This is meant to go into the 4.5.5 release. The v4.5.5 will be the last of the series. It is the perfect time to disable the update mechanism for people running on incompatible Win versions for the next qbt release.

, tr("This is the last supported version for your Windows version.")
, QMessageBox::Ok, this};
msgBox->setAttribute(Qt::WA_DeleteOnClose);
msgBox->setWindowModality(Qt::NonModal);
Copy link
Member

Choose a reason for hiding this comment

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

Why it is non-modal? In this case the user is available to produce multiple similar dialogs at a time.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a copy-paste from here:

if (invokedByUser)

Copy link
Member Author

Choose a reason for hiding this comment

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

The non modal options was implemented with PR #16678.
On second read, that was due to the operation being asyncronous and the user might have opened other dialogs in the meantime.
I'll remove it.

On versions before Windows 10 the user is running the Qt5 build.
The next qBittorrent release will change to Qt6 as the default build.
It is counterintuitive to tell the user about available updates when they are
running an incompatible Windows version.
Copy link
Member

@glassez glassez left a comment

Choose a reason for hiding this comment

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

I don't have Windows on hand to see how it works.
Code LGTM.

@sledgehammer999 sledgehammer999 deleted the noop-update branch July 12, 2023 07:17
@glassez
Copy link
Member

glassez commented Jul 12, 2023

@sledgehammer999
Why did you close it unmerged?

@sledgehammer999
Copy link
Member Author

Why did you close it unmerged?

OMG!
I thought this was merged and I deleted the branch which also closed this. I confused it with the merge of your backport PR.
Damn it.

@sledgehammer999 sledgehammer999 restored the noop-update branch July 18, 2023 00:11
@sledgehammer999
Copy link
Member Author

sledgehammer999 commented Jul 18, 2023

Will it be bad if I merge this and don't wait for translators for the new string (and make a release)?

@glassez
Copy link
Member

glassez commented Jul 18, 2023

Will it be bad if I merge this and don't wait for translators for the new string (and make a release)?

IMO, this string doesn't play a big role in everyday use, so it shouldn't be a big problem to leave it untranslated.
Or you could make a release without this PR, and include it in the next one.

@sledgehammer999
Copy link
Member Author

Or you could make a release without this PR, and include it in the next one.

This release is supposed to be the last one in v4_5_x.

@sledgehammer999 sledgehammer999 merged commit 7104786 into qbittorrent:v4_5_x Jul 18, 2023
32 checks passed
@sledgehammer999 sledgehammer999 deleted the noop-update branch July 18, 2023 11:47
@glassez
Copy link
Member

glassez commented Jul 20, 2023

@sledgehammer999
Shouldn't the similar patch be added in v4.6.x? Even though Qt6 is selected to be used by default, Qt5 is still supported.

@xavier2k6
Copy link
Member

Should below not have been changed to IsWindows10OrGreater too?

const QString OS_TYPE = (::IsWindows7OrGreater() && QSysInfo::currentCpuArchitecture().endsWith(u"64"))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS: Windows Issues specific to Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants