Expose config validation against schema
Closed this issue · 5 comments
Currently, the exposed validate() method is just a placeholder and should be replaced with actual config validation.
Possible functionality of validate():
- It should validate all config files in the config directory
- It should warn or fail when there is no schema file found
- It should provide validation errors on a per-file basis
- It can accept a config file path as string (or array of strings?) which narrows the validation down to one config instead of all.
Stupid question: Do we need to expose validate() at all?
What speaks against keeping it internal and running it as part of load()?
I say yes so it can be part of our users' test suites and CI.j
We're already validating as part of load() if there exists a schema.json, yes.
Ok, what would be a scenario where a user would want to manually call validate() in their tests?
CI I understand a little bit better, but you could also make the case that we haven't used this in our CI so far and I haven't heard anyone really missing this feature or it causing us big pain.
Well, any unit test suite, pre-commit/pre-push hooks, or simply as part of yarn health.
It caused already few prod issues earlier this year when we didn't have our encryption service yet. IMO, we should add a config validation to our own projects as part of CI and yarn health.
To conclude, I think we should both implement and expose a validate([<config>]) function, while still agreeing to the KISS mindset you mentioned elsewhere.
Ok, convinced. Let’s do it :)