search

victorb / trymodule

➰ It's never been easier to try nodejs modules!
1.14k stars 29 forks source link

added standard for linting, husky for triggering the linting on commit #1

Closed capaj closed 8 years ago

victorb commented 8 years ago

Hm, including mocha without any tests does not feel very good. Might as well not include it.

standard I'm all for, happy you included it. However, the node_modules path is too much, just leave it at "pretest": "standard" and npm should figure out the path.

capaj commented 8 years ago

"pretest": "standard" and npm should figure out the path

it doesn't on my system(ubuntu 16.04 with node installed via nvm). Lots of people have the same setup.

Hm, including mocha without any tests does not feel very good. Might as well not include it.

I agree, that is why it would be a good idea to write one

victorb commented 8 years ago

Hm, that's weird, you probably want to open up a issue on npm/npm about the PATH issue. Remember I used to have issues with that as well, but have been fixed recently... Might be an issue with husky otherwise. Let's leave that like that then.

Regarding the tests, the package should have tests but I didn't write them yet since the functionality is very minimal at the moment and takes two seconds to test. A package could be installed, installed since before or a package could not be installed. If you want to write tests for that, go ahead. If not, please remove mocha from the dependencies and I'll merge the PR.

victorb commented 8 years ago

This branch now has conflicts and I'm not sure if you were willing to do the changes so added standard myself.

Tests will be there in the future, once the functionality requires it.

Also, instead of running in a precommit hook and adding another dependency, the "testing" (right now, only linting) is run on CircleCI.

Thanks for raising this PR though :+1: