Skip to content

Conversation

Trott
Copy link
Member

@Trott Trott commented Oct 10, 2021

We currently publish test.js and the GitHub Actions workflow as part of
our npm module. Add a .npmignore file so we don't.

Trott added 2 commits October 9, 2021 22:15
We currently publish test.js and the GitHub Actions workflow as part of
our npm module. Add a .npmignore file so we don't.
@targos
Copy link
Member

targos commented Oct 10, 2021

May I suggest to use an allowlist ("files" in package.json), instead?

@Trott
Copy link
Member Author

Trott commented Oct 10, 2021

May I suggest to use an allowlist ("files" in package.json), instead?

Sure. I'll update this after #98 as it adds a file to the package (format.mjs).

@rvagg
Copy link
Member

rvagg commented Oct 11, 2021

I'm not going to block this, but just want to ask if it really matters? My policy is always to publish everything that makes it a "package", which includes docs and tests (which themselves are a form of docs) so the GitHub repo isn't the only source of truth. If you're including Mb size test fixtures, then sure, but minor test files and whatnot really add no meaningful major burden to package size or complexity.

@Trott
Copy link
Member Author

Trott commented Oct 11, 2021

I'm not going to block this, but just want to ask if it really matters? My policy is always to publish everything that makes it a "package", which includes docs and tests (which themselves are a form of docs) so the GitHub repo isn't the only source of truth. If you're including Mb size test fixtures, then sure, but minor test files and whatnot really add no meaningful major burden to package size or complexity.

I don't feel strongly about it. Another argument for not doing it is that you don't end up accidentally publishing a module with missing files. "Oh, we added format.mjs in the last release but didn't add it to files in our package.json. Oops." The default seems to be "please don't publish tests and stuff" but I don't have strong feelings either way. If anyone feels strongly that we should do this, by all means, re-open.

@Trott Trott closed this Oct 11, 2021
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.

3 participants