Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gofer.getMergedOptions #91

Merged
merged 1 commit into from
Oct 4, 2018

Conversation

jkrems
Copy link
Collaborator

@jkrems jkrems commented Oct 4, 2018

This adds an official way to get the effective options, given some defaults and/or overrides.

@jkrems jkrems force-pushed the jkrems/feature/master/public-prepare-options-3x branch from df51989 to fb9809e Compare October 4, 2018 16:29
@@ -114,13 +123,14 @@ Gofer.prototype._prepareOptions = function _prepareOptions(defaults, options) {
options,
preventComplexMerge
);
delete mergedOptions.endpointDefaults;
Copy link
Member

Choose a reason for hiding this comment

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

Is this supposed to be .endpointName ?

Copy link
Collaborator Author

@jkrems jkrems Oct 4, 2018

Choose a reason for hiding this comment

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

This removes all the other endpointDefaults that do not apply to the selected options. This way it's limited to what would actually have an effect. It's an attempt of reducing the amount of internals this leaks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removing endpointName here would break reporting because we do rely on that setting surviving and making it into our instrumentation layer.

@jkrems jkrems merged commit 709dd59 into master Oct 4, 2018
@jkrems jkrems deleted the jkrems/feature/master/public-prepare-options-3x branch October 4, 2018 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants