Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update worker.js #57

Closed
wants to merge 1 commit into from
Closed

Update worker.js #57

wants to merge 1 commit into from

Conversation

leomp12
Copy link

@leomp12 leomp12 commented Nov 20, 2017

Add support for global npm modules

@gustavnikolaj
Copy link
Owner

It's a very conscious decision that we do not support the globally installed modules. I believe that it is an antipattern and that you're always better off explicitly installing it in your project.

I understand that this is just a fallback case, but I think it will potentially just add to the confusion if you get a wrong version of the linter in case you forget to install your local one.

This is actually mentioned in the readme and we even refer you to another plugin that supports the work flow that you request here. https://github.com/gustavnikolaj/linter-js-standard-engine#what-about-linter-js-standard

Without more contextual information in your PR, I don't think there's any more to discuss. If you want to expand on the reasoning behind adding this flag, please feel free to reopen the PR and post additional information.

@novemberborn I'd also like your input here, if you disagree with the points I made.

@leomp12
Copy link
Author

leomp12 commented Nov 20, 2017

Hello,
In my case it works fine, I keep only one module installed globally, for me it does not make much sense to replicate the installation of the standard in each project, it would be many installations, many updates, more laborious, I prefer to install globally and always use the cli in the same version for all projects.
Actually I did not even keep the package in devDependencies, I inserted just to use the linter in Atom, chances are that on my fork I change it later also, so I do not need to explicit it in package.json.
But of course this is just my case, if it's a consistent decision to keep just locally, fine :)

@leomp12
Copy link
Author

leomp12 commented Nov 20, 2017

It can be a configuration option too ...

@gustavnikolaj
Copy link
Owner

In my case it works fine, I keep only one module installed globally, for me it does not make much sense to replicate the installation of the standard in each project, it would be many installations, many updates, more laborious, I prefer to install globally and always use the cli in the same version for all projects.

If that is the case, I think that the other Standard linter (linter-js-standard) will be a better fit for your usecase. It sounds like you are primarily working on projects alone, which makes it understandable that you haven't experienced any of the issues that guided the design choices we have taken with this linter.

We see an install per project as a massive plus and not as a disadvantage at all, so we are very unlikely to compromise on that. Even introducing it as an option will introduce potential for confusion about which linter you're actually running with, and when we have a suitable other linter that covers the other usecase, I will rule that out. :-)

Actually I did not even keep the package in devDependencies, I inserted just to use the linter in Atom, chances are that on my fork I change it later also, so I do not need to explicit it in package.json.
But of course this is just my case, if it's a consistent decision to keep just locally, fine :)

This sounds exactly like the approach of the linter I referred to before, so I don't think you'll need to maintain your own fork :-)

@sonicdoe
Copy link
Contributor

As a maintainer of linter-js-standard, just a small clarification: linter-js-standard actually won’t use a globally installed standard as well, however, it ships with its own version of standard which is why it works without installing standard as a development dependency.

Having said that, I can perfectly understand your reasoning behind requiring a local standard to be installed 🙂

@gustavnikolaj
Copy link
Owner

@sonicdoe Thanks for the clarification. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants