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

use default options when none provided #29

Closed
wants to merge 1 commit into from

Conversation

ecwyne
Copy link

@ecwyne ecwyne commented Mar 1, 2018

fixes #28

@ecwyne
Copy link
Author

ecwyne commented Mar 1, 2018

@thebigredgeek I wasn't sure if you wanted the dist changes committed or not. Let me know if there are any changes you'd like me to make!

Thanks for an amazing package!

@lxcid
Copy link
Contributor

lxcid commented Mar 27, 2018

What is the reason to have 2 error config though? In constructor I mean

@ecwyne
Copy link
Author

ecwyne commented Mar 27, 2018

const FooError = createError(baseConfig);

throw new FooError(config);

baseConfig provides all of the defaults while config may override or add additional values.

@lxcid
Copy link
Contributor

lxcid commented Mar 27, 2018

Thanks @ecwyne, I figure out on the side though. I came up with a different PR which is mostly identical with yours except with the order the config get merged though, but a lot of it references on the existing PRs.

Thanks for the quick explanation!

This was referenced Mar 28, 2018
@thebigredgeek
Copy link
Owner

thebigredgeek commented Apr 11, 2018

I believe this functionality is now already implemented. I am about to release 1.8 so once you get a chance to upgrade please let me know if this PR is still necessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Message not propagating in 1.7.1
4 participants