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

Improve structure of AddNewTorrentDialog code #20848

Merged
merged 2 commits into from
May 18, 2024

Conversation

glassez
Copy link
Member

@glassez glassez commented May 15, 2024

Code restructuring that separates the basic logic from the logic that depends on the parameters and properties of the torrent being added.

@glassez glassez added this to the 5.0 milestone May 16, 2024
@glassez glassez added GUI GUI-related issues/changes Code cleanup Clean up the code while preserving the same outcome labels May 16, 2024
@glassez glassez marked this pull request as ready for review May 16, 2024 12:58
@glassez glassez requested a review from a team May 16, 2024 12:58
@glassez
Copy link
Member Author

glassez commented May 16, 2024

@qbittorrent/bug-handlers
"Add new torrent dialog" behavior should not be affected.

xavier2k6
xavier2k6 previously approved these changes May 16, 2024
<item>
<widget class="QCheckBox" name="checkBoxNeverShow">
<property name="text">
<string>Never show again</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

When was this functionality removed (doesn't work in master, works in 4.6.4) and why isn't it in the change log?

Copy link
Member Author

Choose a reason for hiding this comment

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

When was this functionality removed (doesn't work in master, works in 4.6.4) and why isn't it in the change log?

@thalieht
Good catch!
It was unintentionally added into this PR.
The fact is that it was originally a preliminary refactoring before other (deeper) changes. But they are still stuck indefinitely, so I decided to merge this commit separately. However, this part of changes doesn't really make sense yet, so I reverted it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well i still don't know if it's meant to be removed eventually but right now it still doesn't work, this is not present in master:

void AddNewTorrentDialog::setEnabled(const bool value)
{
settings()->storeValue(KEY_ENABLED, value);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, it looks like you discovered a regression of #19355 along the way.
Fixed.

thalieht
thalieht previously approved these changes May 16, 2024
@glassez glassez dismissed stale reviews from thalieht and xavier2k6 via a914a6e May 16, 2024 17:41
@glassez glassez requested a review from a team May 16, 2024 17:48
@glassez glassez merged commit b8a774f into qbittorrent:master May 18, 2024
14 checks passed
@glassez glassez deleted the addnewtorrentdialog branch May 18, 2024 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code cleanup Clean up the code while preserving the same outcome GUI GUI-related issues/changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants