Skip to content

fix default values for export #800

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

Merged
merged 1 commit into from
Dec 6, 2021
Merged

fix default values for export #800

merged 1 commit into from
Dec 6, 2021

Conversation

daimor
Copy link
Member

@daimor daimor commented Dec 6, 2021

This PR fixes #799

@daimor daimor requested a review from gjsjohnmurray December 6, 2021 19:32
@daimor daimor merged commit 09dd3f5 into master Dec 6, 2021
@daimor daimor deleted the fix-799 branch December 6, 2021 19:52
@isc-bsaviano
Copy link
Contributor

@daimor Why didn't you include defaults for all of the export settings?

@daimor
Copy link
Member Author

daimor commented Dec 6, 2021

Left, just most of the important and which are not just false by default.

@isc-bsaviano
Copy link
Contributor

Yes but those will be undefined now if they aren't overridden, just like the ones that caused #799. I don't see why you changed the package.json for these settings in #782 and now that we know we need to specify the defaults this way I don't see a good reason to not explicitly set all of the defaults. We should probably delete the old defaults as well since they don't work anymore.

@daimor
Copy link
Member Author

daimor commented Dec 6, 2021

Yeah, I changed it, because I saw some errors, in VSCode logs, something like settings overridden, so, had to put them in correct order. Yeah, probably we would need to delete those defaults, as the useless now. But I see no reasons to put, rarely used parameters in default. With false values, while they supposed to be used as booleans and undefined works as false.

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.

Exporting not working with new version 1.2.1
3 participants