search

browserify / sha.js

Streamable SHA hashes in pure javascript
Other
288 stars 60 forks source link

Test folder should be added to .npmignore #5

Closed vorg closed 7 years ago

vorg commented 10 years ago

Test are needed only for development and shouldn't be distributed on npm where they add 700KB to every library that uses sha.js.

dominictarr commented 10 years ago

This is not an important saving though, because it's not being shipped to a resource constrained destination (eg, the browser). I'd rather leave the tests in.

vorg commented 10 years ago

It's saves time to download your package from npm. I don't ask to remove them. I ask to hide them as there is no value for 99% of the people. The source is still here on github for anybody that needs it. The project i'm working on has dependency tree of 600 libraries event though top level has only 7. So I ended up with 117MB because everybody thinks just another 700KB is ok...

dominictarr commented 10 years ago

It's slow to download from npm because of round trips not because of size. I'm not gonna merge this because I don't think this is the best solution to your problem. You are not gonna save much of that 117 by posting issues on those 600 modules. maybe you shave off a few mb, but if you really want to make a dent, you need to cut down on the number of modules.

Just out of curisoty, which are the 7 top level modules?

vorg commented 10 years ago

brfs 7MB merge 6KB watchify 20MB (19MB of that is browserify) browser-sync 70MB (55MB of cached junk to be removed in next version..) browserify 20MB q 91KB vinyl-source-stream 186KB

No please don't take this issue personally sha.js is one of the smallest offenders on the list. Just had to start somewhere...

dcousens commented 9 years ago

@dominictarr out of interest, does leaving the tests in serve any purpose?

dominictarr commented 9 years ago

I leave my tests in my modules, because I believe that one day, a great hero will arise and do something awesome testing all the modules on npm. If the tests are excluded this can't happen.

dcousens commented 9 years ago

@dominictarr haha

Could they not just download all the git repos instead if they wanted to test against the source? I guess it depends what you think of when you think of npm as a publication platform.

Whether it is literally publication of the source code of the entire package, or if it is just the publication of the code as it will be used.

dominictarr commented 9 years ago

@dcousens the trouble is that the binding between the published package and the git repo is not very strong. it's just a url, what is the actual code? there is a sha1 hash of the tarball, so you could check out the repo with git and then repackage it to check you have exactly the right code. Unfortunately tar archives are not as deterministic as they could be, containing mtime and ctime, so it's not guaranteed that archiving the exact same files produces the exact same tarball.

Remember tar and unix was invented before cryptographic hashes!!!

It would just be way easier to build the everything tester and secure it if the tests are published.

dominictarr commented 9 years ago

or, invent a better file bundler that is more cryptofriendly

dcousens commented 9 years ago

@dominictarr it scares me a little how easy it would be to MITM so many deployments #nsa. I honestly don't think keeping /tests is helping either way though.

The current decision is not in line with the merge here https://github.com/crypto-browserify/browserify-aes/issues/24.

exogen commented 8 years ago

I hope this can be revisited, the sha.js/test dir in my node_modules is by far one of the largest at 1.4MB. It is standard practice to exclude tests from the npm package. While some hypothetical person somewhere might want to peek at them (why wouldn't they just go to the repo?), more likely 99% of sha.js installs are by people who don't even know it's there and don't care about it, it's merely somewhere deep in their dependency tree. It's much better to be respectful of your users' install time and disk space. :)

dcousens commented 8 years ago

I'm personally inclined to agree with @exogen. I'd sooner prefer to just go to the repository (and verify my code matches that repository!) than iterate my node modules.

dominictarr commented 7 years ago

our call

On Thu, Nov 10, 2016 at 1:56 AM, Daniel Cousens notifications@github.com wrote:

@fanatid https://github.com/fanatid sneakily did this in the recent hash-base PR (my bad for not noticing until going back through these issues)... @dominictarr https://github.com/dominictarr do I need to release a patch to put them back?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/crypto-browserify/sha.js/issues/5#issuecomment-259581416, or mute the thread https://github.com/notifications/unsubscribe-auth/AAP1Ls2bDCQ-3PQtUdxYC4F_ClN9hPS4ks5q8nnZgaJpZM4CRf6G .

dominictarr commented 7 years ago

I mean, Your call

On Fri, Nov 11, 2016 at 9:55 AM, Dominic Tarr dominic.tarr@gmail.com wrote:

our call

On Thu, Nov 10, 2016 at 1:56 AM, Daniel Cousens notifications@github.com wrote:

@fanatid https://github.com/fanatid sneakily did this in the recent hash-base PR (my bad for not noticing until going back through these issues)... @dominictarr https://github.com/dominictarr do I need to release a patch to put them back?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/crypto-browserify/sha.js/issues/5#issuecomment-259581416, or mute the thread https://github.com/notifications/unsubscribe-auth/AAP1Ls2bDCQ-3PQtUdxYC4F_ClN9hPS4ks5q8nnZgaJpZM4CRf6G .