search

aseemk / requireDir

Node.js helper to require() directories.
MIT License
484 stars 60 forks source link

ignore node_modules #11

Closed haoxins closed 9 years ago

haoxins commented 9 years ago

the case is requireDir with recurse: true, and there are node_modules exist.

haoxins commented 9 years ago

ping @aseemk

haoxins commented 9 years ago

@aseemk any advice?

aseemk commented 9 years ago

Sorry for missing this! Will TAL.

aseemk commented 9 years ago

Thanks for doing this!

My only major piece of feedback is that instead of handling this at the top of the requireDir function, we should instead handle it where we recurse. i.e. Don't recurse if base === 'node_modules'.

(Note that base has already been derived there too.)

The advantage of that is it'll support an explicit requireDir('./node_modules'), and it won't return an empty node_modules entry if we're skipping it.

It'd also be nice if there were a test for this, and also if the readme were updated to mention this. I don't mind doing these things, but might not be able to get to it right away.

Finally, ideally, this would be an option, just set to the default. But perhaps it makes sense to do a more general ignore option instead, so I don't mind not worrying about the option for now.

haoxins commented 9 years ago

@aseemk done

ci failed on node@0.8

how about rm 0.8 and add 0.12, iojs?

aseemk commented 9 years ago

Great work, and thanks! Happy to merge after these final changes.

haoxins commented 9 years ago

everything should be OK :smile:

aseemk commented 9 years ago

Published to npm as v0.2.0. Thanks again!