Skip to content

fix: bug/issue 25 #94

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

Merged
merged 2 commits into from
Apr 22, 2022
Merged

fix: bug/issue 25 #94

merged 2 commits into from
Apr 22, 2022

Conversation

in03
Copy link
Owner

@in03 in03 commented Apr 22, 2022

Ensure all required settings exist.
Allow substitution with defaults or custom values at runtime.

in03 added 2 commits April 5, 2022 15:39
This is really a final enhancement to the #25 'Handle Missing Keys'
issue. We can now substitute missing settings with default app values,
or we can type a new custom value in, all on initial app startup.

FUTURE CONSIDERATIONS:
If configuration management were to become more important in future,
the following may need to be addressed:

- If a whole section is missing with an inline comment, a different
function parses the diff to the usual. Some of this code could be better
shared with the main function. There's currently minor inconsistency.

- Missing settings are declared as `root['app']['loglevel']`. This could
be more human readable. e.g. `app -> loglevel`

- Expected custom values are not parsed or validated properly. They're
just taken as is.

- If a whole setting section is missing. The default value is an
ordereddict of all the missing nested key/vals within.
This isn't human readable and it's **definitely** not possible to type
out a single custom value to represent that whole structure,
instead each nested key/val within should be iterated one-by-one.

- This would all be necessary if we wanted to add a config command to
the CLI, but surely YAML is easy enough.
@in03 in03 merged commit 0e43505 into main Apr 22, 2022
@in03 in03 deleted the bug/issue-25 branch April 22, 2022 04:21
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.

1 participant