search

ditinc / linter-cflint

:shirt: CFLint plugin for Atom Linter
MIT License
4 stars 7 forks source link

Do not use execSync #3

Closed steelbrain closed 9 years ago

steelbrain commented 9 years ago

It's a bad practice because execSync blocks the event loop completely until the child process is completed, linter's API allows you to return a promise and resolve it with results, You could use that.

sjmatta commented 9 years ago

Thanks! It seems to work a lot smoother now.

steelbrain commented 9 years ago

Side note: You can use Helpers::exec instead of adding a completely new dependency, also it provides a nicer API. and by helper I mean the atom-linter npm module

sjmatta commented 9 years ago

I appreciate the feedback! I reduced the external dependencies and used atom-linter instead.

steelbrain commented 9 years ago

Also, I am afraid you are not using atom-package-deps correctly, You are supposed to add both of those deps to your package manifest (aka package.json) and then call atom-package-deps::install using current package's name only once, which would look like require('atom-package-deps').install('linter-cflint')

sjmatta commented 9 years ago

Thanks for your help- I think I have it correct now.