jslicense / licensee.js

check dependency licenses against rules
https://www.npmjs.com/package/licensee
Apache License 2.0
185 stars 23 forks source link

Add gitignore and npmignore #58

Closed brettz9 closed 4 years ago

kemitchell commented 4 years ago

@brettz9 if you do Node/JavaScript development, I'd strongly recommend you put user_modules in user-global .gitignore.

This package probably should exclude tests from distribution tarballs. Would you like to land that PR yourself, or should I go ahead and do it?

brettz9 commented 4 years ago

Sorry, are you asking to just add user_modules (along with node_modules) in .gitignore?

I am behind the Great Firewall in China, and can't get around it atm, so not able to Google user_modules, but thanks for the tip.

kemitchell commented 4 years ago

I do:

echo >> ~/.gitconfig <<EOS
[core]
excludes_file = ~/.gitignore_global
EOS
echo node_modules >> ~/.gitignore_global
ljharb commented 4 years ago

I’m not familiar with user_modules; node_modules should be gitignored and i believe needn’t be npmignored since npm will always ignore it unless bundledDependencies are specified.

kemitchell commented 4 years ago

user_modules was a typo. Might've been my phone.

brettz9 commented 4 years ago

@kemitchell : Re: global gitignore, while I understand the appeal, I think the advantage of a local one is consistency among project developers. Of the good many projects I've submitted PRs for, I've come across quite a few who have strong opinions about whether to include or exclude package-lock.json and such, but I don't think I've come across any others who expected a .gitignore_global. But if you don't want it, I can amend to only include the .npmignore in this PR.

kemitchell commented 4 years ago

@brettz9 I don't think we need an .npmignore. The package.json file for this package uses a files array. And indeed there aren't any test files in the latest tarball:

mkdir tmp
cd tmp
npm i -D licensee
ls node_modules/licensee

I hear you on .gitignore. The argument that got me years ago boils down to this: Should nearly npm package project have node_modules in .gitignore? If so, and I think so, then node_modules should be ignored by default, rather than cluttering every repository with the same .gitignore.

brettz9 commented 4 years ago

Re: npmignore, yeah, sorry, I seem to have a mental block in forgetting to check files.

Re: gitignore, assuming Git, which is of course not Node specific and has no mechanism for detecting the language, would agree to such a default, it could still cause problems with those using bundledDependencies who actually include node_modules in their repositories.

kemitchell commented 4 years ago

git add --force node_modules overrides the default.

brettz9 commented 4 years ago

So where is your proposal to Git to allow node_modules by default, so we can all benefit from this? :-)

kemitchell commented 4 years ago

It's a user-space thing.

https://help.github.com/en/github/using-git/ignoring-files#create-a-global-gitignore

https://github.com/github/gitignore/blob/master/Node.gitignore#L41

brettz9 commented 4 years ago

I guess it is a reasonable practice and even if it causes me to forget adding it where explicit .gitignore files are needed, I can justify it, since it is really hardly project-specific for Node users. Thanks!