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

Move old RSS items to separate config file. Closes #6167. #6175

Merged
merged 1 commit into from
Jan 22, 2017

Conversation

magao
Copy link
Contributor

@magao magao commented Jan 1, 2017

This is one of a series of pull requests designed to improve the responsiveness of the UI. This pull request addresses #6167.

When editing RSS downloader rules, each edit results in persisting the rules to disk. Since the old_items are in the same file, this also means persisting the old_items to disk each time a rule is edited. I have ~100 rules which use <1MB on disk, but the old_items use ~70MB, so persisting them each edit affected responsiveness.

This change moves the RSS old_items from qBittorrent-rss.conf to a new qBittorrent-rss-items.conf. Existing entries are migrated on first run.

@magao
Copy link
Contributor Author

magao commented Jan 8, 2017

Rebased on top of #6212 and uncrustified.

@sledgehammer999 sledgehammer999 added the RSS RSS-related issues/changes label Jan 18, 2017
}
else {
allOldItems = qBTRSS.value("old_items", QHash<QString, QVariant>()).toHash();
}
Copy link
Member

@sledgehammer999 sledgehammer999 Jan 18, 2017

Choose a reason for hiding this comment

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

move the migrating code under a function called "migrateRSS()" in the file `src/app/upgrade.h".
Then call it inside main() at the appropriate position.
Wrap all that code in:

#ifndef DISABLE_GUI
// do stuff
#endif

After all this the code in this file will be much cleaner.
And in the future, we have to look at one place to remove old upgrading/migrating code.

@sledgehammer999
Copy link
Member

Also doesn't old_items hold the RSS feeds actually? Then why not name the new file qBittorrent-rss-feeds?

@magao
Copy link
Contributor Author

magao commented Jan 18, 2017

qBittorrent-rss-feeds works for me - I called it qBittorrent-rss-items because it held the old_items config option.

@magao
Copy link
Contributor Author

magao commented Jan 21, 2017

@sledgehammer999 Rebased PR on top of master and addressed review comments.

@@ -258,4 +258,23 @@ void macMigratePlists()
#endif // Q_OS_MAC


Copy link
Member

Choose a reason for hiding this comment

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

don't leave 2 newlines

Copy link
Member

Choose a reason for hiding this comment

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

I mean that 1 newline is fine.

}
#endif


Copy link
Member

Choose a reason for hiding this comment

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

same here

@magao
Copy link
Contributor Author

magao commented Jan 22, 2017

@sledgehammer999 Fixed double-newlines, including existing doubled newlines before migratePlistToIni.

@sledgehammer999
Copy link
Member

Thank you.

@sledgehammer999 sledgehammer999 merged commit 077ad65 into qbittorrent:master Jan 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RSS RSS-related issues/changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants