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

Update npm-ignore rules for all npm published packages #824

Closed
wants to merge 5 commits into from

Conversation

usergenic
Copy link
Contributor

  • Normalized all the .npmignore files to standard form with leading / and removed unnecessary automatic exclusions like node_modules and missing files like .clang-format.
  • Updated CHANGELOGs for packages which had .npmignore changes so the next release picks them up.
  • Opportunistic clean-up here in that I also removed unused per-package-top-level files like .github and appveyor.yml.
  • One or two .js files may have had formatting slightly updated because clang-format.

Here's a gist of the results of the new ignores: https://gist.github.com/usergenic/04c04c01623c0c8342bdbfb336cfdf63

 - We published our test files and demo files in some packages.
 - We published our source files in some packages.
 - We published our config files in some packages.
Copy link
Contributor

@rictic rictic left a comment

Choose a reason for hiding this comment

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

This seems reasonable, but it's tricky to be certain that it's correct or to test that it's correct, and it can break a release if it's wrong.

@usergenic
Copy link
Contributor Author

@rictic you make a good point. Probably best way to visualize the effects of changes and mitigate risk is to do packages as separate PRs including a diff of npm packaged files in the descriptions.

@rictic
Copy link
Contributor

rictic commented Dec 21, 2018

Yeah, that'd be way easier to review! Sorry for the added friction!

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

Successfully merging this pull request may close these issues.

3 participants