Merge provided context with default context #1953
Conversation
|
7c5abe6 did not affect testing - is this something we should be worried about? Also, build not failing due to my changes - appears to be timeouts. |
|
|
||
| //Merge the two contexts, using the provided context as source to override the default if requried. | ||
| if (context) { | ||
| Hoek.merge(defaultContext, context); |
There was a problem hiding this comment.
merge() is not the right tool here because we don't want to create a deep copy of context. Instead we need a shallow copy of the keys. A simple Object.keys(context) with a for loop would be enough.
|
Have fixed as you suggested |
|
|
||
| //Merge the two contexts, using the provided context as source to override the default if requried. | ||
| if (context) { | ||
| Object.keys(context).forEach(function(key){ |
There was a problem hiding this comment.
We don't allow functional programming in live code (e.g. outside of configuration or startup). It's too slow.
There was a problem hiding this comment.
Sorry, not sure I understand what you mean? How should I change this?
There was a problem hiding this comment.
use a for loop instead of forEach.
|
Implemented in vision. |
|
This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions. |
This addresses issue #1950
My first commit to Hapi so hopefully all is as required. Have included two test cases to ensure this works as expected - 100% test coverage.
Chris