[refactor] Client Config: Pass path around to prepare for extraction from Config
#11860
Conversation
| @@ -149,7 +151,7 @@ public ApplicationSettings(PersistentConfig persistentConfig, Config config, UiC | |||
| .Where(value => value && string.IsNullOrEmpty(LocalBitcoinCoreDataDir)) | |||
| .Subscribe(_ => LocalBitcoinCoreDataDir = EnvironmentHelpers.GetDefaultBitcoinCoreDataDirOrEmptyString()); | |||
|
|
|||
| // Apply RunOnSystenStartup | |||
| // Apply RunOnSystemStartup | |||
| else | ||
| { | ||
| return Money.Parse(stringValue); | ||
| } |
There was a problem hiding this comment.
Unrelated. Yet simpler.
| ConfigFilePath = Path.Combine(Config.DataDir, "Config.json"); | ||
| Directory.CreateDirectory(Config.DataDir); |
There was a problem hiding this comment.
Moved here from the LoadOrCreateConfigs method.
kiminuo
changed the title
Config: Pass path around to prepare for extraction from Config.Config: Pass path around to prepare for extraction from Config
| @@ -122,7 +122,7 @@ private static IClientConfig CreateConfig() | |||
|
|
|||
| private static IApplicationSettings CreateApplicationSettings() | |||
| { | |||
| return new ApplicationSettings(Services.PersistentConfig, Services.Config, Services.UiConfig); | |||
| return new ApplicationSettings(Services.PersistentConfigFilePath, Services.PersistentConfig, Services.Config, Services.UiConfig); | |||
There was a problem hiding this comment.
Just pass the config file path on.
| { | ||
| _startupConfig = new PersistentConfig(persistentConfig.FilePath); | ||
| _persistentConfigFilePath = persistentConfigFilePath; | ||
| _startupConfig = new PersistentConfig(persistentConfigFilePath); |
There was a problem hiding this comment.
The important thing here is that we do not access the config path using persistentConfig.FilePath but rather persistentConfigFilePath. That's what we want.
| Config = Guard.NotNull(nameof(config), config); | ||
| ExecuteWhenChanged = Guard.NotNull(nameof(executeWhenChanged), executeWhenChanged); |
There was a problem hiding this comment.
Unrelated but a trivial change to remove unneeded code (plus if one wants it makes it easier to unit test this class actually).
👍
I don't think we need to get rid of all Footnotes
|
Currently
Configclass hasFilePathproperty but it has downsides:This PR passes the path for persistent config separately and avoids all these issues. The PR is meant to be a precursor for #11764 and to slowly move towards a better design.
No change in behavior is intended.