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] npmignore updated with file list #232

Merged
merged 2 commits into from
Aug 25, 2018
Merged

Conversation

nutboltu
Copy link
Collaborator

Fixes #231

@nutboltu nutboltu self-assigned this Aug 24, 2018
@coveralls
Copy link

coveralls commented Aug 24, 2018

Coverage Status

Coverage remained the same at 93.364% when pulling 32504c1 on feature/npmignore into 2a50547 on master.

@superhit0
Copy link
Collaborator

# ignore all

*

# override above and include only dist folder

!dist/**/*

@nutboltu @patw0929

How about this approach?

to me it looks cleaner

@nutboltu
Copy link
Collaborator Author

@superhit0 shouldn't we include LICENSE, package.json, CHANGELOG.md etc. to give the user more context about the library?

@superhit0
Copy link
Collaborator

LGTM from me.

@patw0929 can you confirm on this ?

@patw0929
Copy link
Owner

According to npmjs doc:

The following paths and files are never ignored, so adding them to .npmignore is pointless:

package.json
README (and its variants)
CHANGELOG (and its variants)
LICENSE / LICENCE

Can you help to confirm it? Thanks!

@nutboltu
Copy link
Collaborator Author

@patw0929 Thanks for the feedback. I didn't know that .npmignore never ignore these files. I updated the PR.

Copy link
Owner

@patw0929 patw0929 left a comment

Choose a reason for hiding this comment

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

LGTM!

@superhit0 superhit0 merged commit 14accbe into master Aug 25, 2018
@nutboltu nutboltu deleted the feature/npmignore branch October 5, 2019 10:51
andrewsantarin pushed a commit to andrewsantarin/react-intl-tel-input that referenced this pull request Feb 2, 2022
npmignore updated to include only built files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants