Skip to content

Post-v0.0.1 Fixes - Release v0.0.2 #14

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 8 commits into from
Jan 29, 2021
Merged

Post-v0.0.1 Fixes - Release v0.0.2 #14

merged 8 commits into from
Jan 29, 2021

Conversation

xhluca
Copy link

@xhluca xhluca commented Jan 29, 2021

Making this PR to add a few things I noticed were missing. Will be shipped in v0.0.2

@xhluca
Copy link
Author

xhluca commented Jan 29, 2021

@plotly/dash-core Wanted to have your thoughts on the .npmignore part; I noticed that dash core components only publish the JS files to npm.

image

But the components built with the boilerplate seems to also publish a bunch of irrelevant files when I run npm publish:

image

Should we just manually add all the files to npmignore, or would it be better to use some "include" scripts (similar to manifest.in)?

@jourdain
Copy link
Contributor

Yes update .npmignore to only push the generated + src of the JS with the .map file as well.

.npmignore Outdated
.circleci
.vscode
.github
dist
Copy link
Contributor

Choose a reason for hiding this comment

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

dist should be part of the npm package

Copy link
Author

Choose a reason for hiding this comment

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

Does dist/ contain any .js file? I was under the impression only the generated wheels goes there

Copy link
Author

Choose a reason for hiding this comment

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

does dist contain any JS file? I thought only the wheels go there (might be wrong)

Copy link
Author

Choose a reason for hiding this comment

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

Actually, might be a better idea to use the files field: https://docs.npmjs.com/cli/v6/configuring-npm/package-json#files

Copy link
Contributor

Choose a reason for hiding this comment

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

You don't want to ignore the js files that should be part of the npm package. no need to add then in package.json in files.

@xhluca xhluca changed the title Post-v0.0.1 PR Post-v0.0.1 Fixes - Release v0.0.2 Jan 29, 2021
@xhluca
Copy link
Author

xhluca commented Jan 29, 2021

@jourdain Just realized the utils directory is not included (since we didn't specify it in setup packages). I'd like to merge this PR and release v0.0.2 asap so we can have the utils. Let me know if there's anything I should change first.

@xhluca xhluca merged commit 28d345a into master Jan 29, 2021
@xhluca xhluca deleted the post-pr branch January 29, 2021 23:38
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.

2 participants