Make sure that multiple setModuleDefaults calls update the config sources properly#825
Make sure that multiple setModuleDefaults calls update the config sources properly#825jdmarshall wants to merge 1 commit intonode-config:masterfrom
Conversation
config and the sources being consistent with each other.
|
I'd like us to sort out the setModuleDefaults issue. I think I may have a solution we could to in Util to keep the defaults and the loaded config as two separate pieces of data and do the heavy lifting in setModuleDefaults so that it's a zero cost abstraction if nobody is using this feature. Since it's a startup only operation I'm a little less concerned about the perf. Plus I just landed some perf improvements anyway. I have another idea for fixing it but it might introduce a circular dependency so I need to think on it more: |
markstos
left a comment
markstos
left a comment
There was a problem hiding this comment.
Looks OK, but let's not merge it ahead of the regression fix, since it's a breaking change and the regression fix may go out before 4.1.
QA Log
- Reviewed diff
- Noted extra test coverage.
|
@markstos So I think we are probably ready for 4.0.1 |
|
4.0.1 is OK with me. |
|
this conflicts a bit with a PR for #822 which also happens to fix this one. |
2 participants
I don't know if this constitutes a breaking change. I don't know if anyone is relying on configSources for programmatic behavior.
Abundance of caution says yes.
fixes #827