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

fix: make the default config work properly #309

Closed
wants to merge 2 commits into from

Conversation

nytamin
Copy link

@nytamin nytamin commented Aug 1, 2019

This is a fix for #308

After
2fb83f5#diff-6eec2f2dfcb5bb90afa10af53927034bR83
the "git"-property will always exist on the config-object, even if the value of it is undefined.

In that case, the Object.assign() in lib/index.js:52 will not pick the default property, but instead the (undefined) property of the config-object.

This PR fixes the issue by removing all undefined properties from the config object, allowing the default-properties to properly take over.

nytamin added 2 commits August 1, 2019 15:23
If a property in the config-object exists but is undefined, the Object.assign on the following line will not pick the default property, but instead the (undefined) property of the config-object.
Copy link

@RDIL RDIL left a comment

Choose a reason for hiding this comment

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

please merge!

@nytamin
Copy link
Author

nytamin commented Aug 8, 2019

@tschaub any updates on this? Do you need anything more before merging this fix?

@tschaub
Copy link
Owner

tschaub commented Aug 8, 2019

Thanks for the proposed change and for bringing this to my attention. The [email protected] release includes a fix.

@tschaub tschaub closed this Aug 8, 2019
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.

3 participants