Skip to content

Ignore test and examples in the npm module #43

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 1 commit into from
Jan 29, 2016

Conversation

hmalphettes
Copy link
Contributor

It makes the tarred-gzipped module smaller by a factor of 69: from 331K to 4.8K

Not sure if anybody cared. One of my team mates actually felt strongly about it: after our own efforts to not package csv files in our own codebase we could still find those hanging in there.

Thanks again for all the csv-* modules!

mafintosh added a commit that referenced this pull request Jan 29, 2016
Ignore test and examples in the npm module
@mafintosh mafintosh merged commit 65136b8 into mafintosh:master Jan 29, 2016
@mafintosh
Copy link
Owner

TIL .npmignore - Thanks!

@mafintosh
Copy link
Owner

/cc @feross, re our earlier conversation

@mafintosh
Copy link
Owner

Released in 1.9.3

@feross
Copy link

feross commented Jan 29, 2016

I personally don't think it's a good idea to ignore the whole test/ folder. That's documentation for your module that will now be missing when you're offline and need to look inside node_modules/csv-parser.

Ignoring huge test files like those in test/data is a good idea, but I don't think ignoring test/test.js (which is only 8kb before gzipping) is a good tradeoff.

Previous discussions:

nodejs/readable-stream#100
rvagg/through2#36
browserify/browserify#1093
browserify/sha.js#5
feross/string-to-stream#1
beatgammit/base64-js#16

@feross
Copy link

feross commented Jan 29, 2016

Another annoying issue is that .npmignore overrides .gitignore, so now they need to manually be kept in sync.

Without a .npmignore, npm will just use the .gitignore. But if there's an .npmignore it uses that instead. So if you add something to .gitignore but forget to add it to .npmignore, you're now publishing that to npm, even though you don't see it pushed to GitHub. This bit me once when I published a password to npm by accident and didn't notice for a very long time.

@mafintosh
Copy link
Owner

good points @feross

@hmalphettes
Copy link
Contributor Author

@feross @mafintosh: I convinced @mafintosh to not have "large" csv files in the packaged module and but I remove way more. It was not my intention, sorry about that!

Working on packaging nodejs software I am affected by the current DevOps frenzy of making images really small for production use: faster deployments and less unknowns when there is less code.

Regarding maintaining .gitignore and .npmignore: I dont think we can solve that issue here.
Should we suggest them to support inheriting .gitignore and search first that it has been suggested already ?

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