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

Decision: Allow invalid config settings, automatically use default setting in their place #166

Open
atc0005 opened this issue Nov 22, 2019 · 1 comment
Assignees
Labels
meta: design High-level design considerations; related to feature or changes that require further review question Further information is requested
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented Nov 22, 2019

While working on #161 I dealt with a case (pun intended) where I forgot to deal with a "stop the world" error and the code path descended into a switch statement which assumed only valid values would be provided.

This could be worked around in multiple ways, but these appear to be the most "accessible" for my current skill level:

  • panic immediately upon finding an invalid value, even if it would otherwise be overridden later by another config source
    • this might be tricky for the current state of the code
  • panic at the end after merging all configuration sources (file, env vars, flags) and finding invalid values
  • emit warnings, then replace invalid values with valid default values

For now, I'll plan to implement early failure/panic when finding invalid values after merging all config sources. This can either be the final behavior or can be enhanced by early failure upon first encountering an invalid value.

@atc0005 atc0005 added question Further information is requested meta: design High-level design considerations; related to feature or changes that require further review labels Nov 22, 2019
@atc0005 atc0005 added this to the Future milestone Nov 22, 2019
@atc0005 atc0005 self-assigned this Nov 22, 2019
@atc0005
Copy link
Owner Author

atc0005 commented Mar 15, 2021

  • emit warnings, then replace invalid values with valid default values

I should use the principle of least surprise. Having an application assume what I wanted to say when I didn't say it is a dangerous default, especially for an application in charge of pruning files. This is a no-go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta: design High-level design considerations; related to feature or changes that require further review question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant