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

Migrate configurations from bug fixed in #11173. Fixes #11313 #11347

Merged
merged 5 commits into from
Mar 30, 2021

Conversation

NinjaLikesCheez
Copy link
Contributor

This change adds a configuration migration to address potential configurations addressed in #11173 by allowing the PerformMigration function of ConfigurationService to take a path to a previous configuration directory, and adding a check in CreateOrMigrateSettings for a misconfiguration.

It also adds a test case for RuntimeSettings.DataFolder to ensure the default configuration path matches expectations.

@NinjaLikesCheez
Copy link
Contributor Author

Hm, took a swing at blindly hoping I got the Window test right, going to try with Environment.ExpandEnvironmentVariables as that looks like an appropriate solution.

@NinjaLikesCheez
Copy link
Contributor Author

I'm an idiot and forgot that Windows uses \ as the file system separator...

@ilike2burnthing ilike2burnthing requested a review from a user March 19, 2021 01:40
@garfield69 garfield69 requested a review from ngosang March 21, 2021 02:50
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Other than the non-blocking comments I made, I can't see anything wrong with this code.

src/Jackett.Common/Services/ConfigurationService.cs Outdated Show resolved Hide resolved
src/Jackett.Common/Services/ConfigurationService.cs Outdated Show resolved Hide resolved
src/Jackett.Common/Services/ConfigurationService.cs Outdated Show resolved Hide resolved
NinjaLikesCheez and others added 2 commits March 30, 2021 16:57
* remove dead code
* invert existance check in PerformMigration
@garfield69
Copy link
Contributor

Okely Dokely.
Well since we are unlikely to get a review from ngosang in a timely manner, I'll merge this based on the review by XYZJR .
Thank you.

@garfield69 garfield69 merged commit f5688f2 into Jackett:master Mar 30, 2021
@NinjaLikesCheez
Copy link
Contributor Author

Thank you both for the reviews and assistance :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants