search

jscs-dev / node-jscs

:arrow_heading_up: JavaScript Code Style checker (unmaintained)
https://jscs-dev.github.io
MIT License
4.96k stars 511 forks source link

Configuration: exclude nested node_modules by default #2122

Closed kirbysayshi closed 8 years ago

kirbysayshi commented 8 years ago

Hello there!

I had a project with a structure similar to this:

- /
  - src
    - file1.js
    - file2.js
  - inner-project
    - node_modules
    - package.json
  - node_modules
  - package.json

I ran the CLI, and it hung. I looked at the docs, which stated that node_modules is excluded by default. But then I wondered if inner-project's node_modules were being included, and it appears they were!

I figured this change might make it easier for new users, even though this is a slightly non-standard project layout. I also wondered if I should add a * to .git for the case of older git submodules, but not sure.

I understand if this is not really inline with the project needs, so I won't be worried if this isn't approved or is discarded.

Thanks for a great tool!

hzoo commented 8 years ago

I think we can do this but it's usually better if you define the folder's you are linting more explicitly instead of jscs . (if you are actually doing that).

Like jscs folders/*/src or something?

kirbysayshi commented 8 years ago

@hzoo I agree with defining specific folders to lint, but even if I did it the nested node_modules would still be present and still be a problem (if I'm understanding you correctly). In my project, I control the code in the inner-project folder, but it has separate dependencies.

Perhaps a better example project where this could be an issue:

- client
  - node_modules
  - package.json
- server
  - node_modules
  - package.json
- package.json // just contains npm scripts, for example

And of course, a person can work around this by adding */node_modules/** to their excludeFiles config, but maybe this makes it even easier to use jscs to have it by default?

kirbysayshi commented 8 years ago

I'm unfamiliar with AppVeyor, but the build failure doesn't seem related to my change. Is it, and I'm just missing something?

hzoo commented 8 years ago

Ah don't worry about that (yeah it's not related). Also i'm not sure why travis isn't running (it was down before)

kirbysayshi commented 8 years ago

ok cool! thanks for letting me know.

markelog commented 8 years ago

Thank you!