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 #429

Closed
wants to merge 1 commit into from
Closed

Update .npmignore #429

wants to merge 1 commit into from

Conversation

udivankin
Copy link

exclude .editorconfig .eslintignore .eslintrc from bundling to npm package

exclude .editorconfig .eslintignore .eslintrc from bundling to npm package
Copy link
Owner

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

no, these are intentionally included so that npm explore qs && npm install && npm test always works.

@ljharb
Copy link
Owner

ljharb commented Jan 12, 2022

Duplicate of #400. Duplicate of #389. Duplicate of #356. Duplicate of #250.

The only files that currently show up in npm pack --dry-run that could be excluded are .github/FUNDING.yml - every other file is 100% necessary in the published package.

@ljharb ljharb closed this Jan 12, 2022
@udivankin udivankin deleted the patch-1 branch January 12, 2022 17:32
@udivankin
Copy link
Author

Well, I've read #250 and it still feels a bit weird for me.
What about putting a short notice in .gitignore explaining why these were intentionally opted out

@ljharb
Copy link
Owner

ljharb commented Jan 12, 2022

Given that there's an explicit .npmignore file, it seems like a reader should expect that there is a reason without any additional comment, instead of assuming that it's a mistake.

The presence of the .npmignore file hasn't stopped people from filing PRs with the "files" field, and I suspect a comment in either .gitignore or .npmignore would also not achieve the desired effect :-/

@ljharb ljharb mentioned this pull request May 6, 2024
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