Closed mikabytes closed 6 months ago
@lencioni If it's convenient for you, would you mind having a look at this code?
Or, if you're pressed for time, perhaps I should go ahead and merge it. Should be fairly safe.
Thanks
Thanks for doing this! I reviewed on a phone so sorry about any weird formatting
@lencioni Thanks for your feedback, much appreciated!
I put some comments of my own. I'll wait for you to review them, and the changes, before merging this.
Our configuration schema is big and complex. This is mostly due to every property having the ability to also be a function (dynamic configuration).
Several properties don't belong in configuration. We might want to look into extracting them at a later point. Examples are
cacheLocation
,coreModules
.. These are also undocumented. They bring in a hairball of dependencies that don't make sense for a schema to deal with. It would clean things up a bit to refactor them out.This implementation will return messages on bad configuration and delete the offending keys. Alternatively, we could crash, but seeing as we have earlier ignored them this seems to be the most reasonable way to go.
Changed one test using Set to use Array instead, as I couldn't validate Sets using
instanceof
. Not sure why. But it works.