Skip to content

Merge provided context with default context#1953

Closed
chriswiggins wants to merge 4 commits intohapijs:masterfrom
chriswiggins:hapi-keep-default-context
Closed

Merge provided context with default context#1953
chriswiggins wants to merge 4 commits intohapijs:masterfrom
chriswiggins:hapi-keep-default-context

Conversation

@chriswiggins
Copy link
Copy Markdown

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

@chriswiggins
Copy link
Copy Markdown
Author

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.

lib/views.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@hueniverse hueniverse added the feature New functionality or improvement label Sep 16, 2014
@hueniverse hueniverse self-assigned this Sep 16, 2014
@chriswiggins
Copy link
Copy Markdown
Author

Have fixed as you suggested

lib/views.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't allow functional programming in live code (e.g. outside of configuration or startup). It's too slow.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, not sure I understand what you mean? How should I change this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use a for loop instead of forEach.

@hueniverse
Copy link
Copy Markdown
Contributor

Implemented in vision.

@lock
Copy link
Copy Markdown

lock bot commented Jan 9, 2020

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants