Skip to content
This repository has been archived by the owner on Jul 12, 2022. It is now read-only.

File settings #414

Merged
merged 10 commits into from
Sep 7, 2018
Merged

Conversation

AnthonySteele
Copy link
Member

Config file for persisting commandline settings
#339

@AnthonySteele
Copy link
Member Author

AnthonySteele commented Sep 3, 2018

Consider this a proof of concept. So far the file format is simple, json file containing key-value pairs. The keys should all be the same as the long form of a command line switch. Capitalisation of keys should not matter.

e.g.

{
  "age": "8d",
  "api": "https://github.contoso.com/api/v3",
  "exclude": "^Microsoft.AspNetCore"
}

These can be filled out with other settings now without too much trouble.

Actual command line switches, if present, will override what's in the file.

What I want to sort out soon is the rest of it - the file name, the file path(s) that are checked and does this work correctly when the global tool is run, how the code parses it. The current code is OK but not great.

It is possible to have a commandline arg to use a named file, e.g. --settingsfile c:\some.file.json but I would leave that as a seperate feature in a later PR.

@AnthonySteele
Copy link
Member Author

AnthonySteele commented Sep 6, 2018

Work has got further, there are now 5 string values in the config file: "age", "api", "include", "exclude", and "label".
And a test on reading it.

@AnthonySteele
Copy link
Member Author

AnthonySteele commented Sep 7, 2018

I have had a play with doing this via the .NETCore settings api instead of Newtonsoft and file reading.

It's a simple replacement, entirely inside FileSettingsReader. The code looks like this:

        private FileSettings ReadFile(string folder, string fileName)
        {
            try
            {
                var builder = new ConfigurationBuilder()
                    .SetBasePath(folder)
                    .AddJsonFile(fileName);

                var settings = builder.Build();
                return settings.Get<FileSettings>();
            }
            catch (Exception ex)
            {
                _logger.Error($"Cannot read setings file at {folder} {fileName}", ex);
                return FileSettings.Empty();
            }
        }

Pro: This opens the door to easily reading from more than one file, env vars etc using the fallback mechanisms in the config api.

Con: This needs three new packages added to the project: Microsoft.Extensions.Configuration, Microsoft.Extensions.Configuration.Binder and Microsoft.Extensions.Configuration.FileExtensions

Deciding factor: YAGNI. Not going to change over to settings api now as don't need it to only read 1 file. Cons outweigh it, and it can be added later if need be. Doing this later if necessary seems not hard - it would be a change in 1 class, FileSettingsReader, and will not ripple through the codebase.

@AnthonySteele AnthonySteele merged commit 91217ba into NuKeeperDotNet:master Sep 7, 2018
@AnthonySteele AnthonySteele deleted the file-settings branch September 21, 2018 09:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants