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

Docs: Add npx usage to Getting Started guide #11249

Merged
merged 3 commits into from Jan 16, 2019
Merged

Conversation

eyalch
Copy link

@eyalch eyalch commented Jan 7, 2019

npx is a shorter alternative to navigating to the executable found in node_modules/.bin

What is the purpose of this pull request? (put an "X" next to item)

[X] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)
I've added a documentation regarding the usage of eslint with npx.

Is there anything you'd like reviewers to focus on?

`npx` is a shorter alternative to navigating to the executable found in `node_modules/.bin`
@jsf-clabot
Copy link

jsf-clabot commented Jan 7, 2019

CLA assistant check
All committers have signed the CLA.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Jan 7, 2019
Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general, just one request: Could you please note when npx is available (any minimum Node or npm version)? If all of those minimum versions are below what ESLint supports, then we don't need to add that info to the text here.

Thanks for contributing!

@platinumazure platinumazure added documentation Relates to ESLint's documentation accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jan 7, 2019
@eyalch
Copy link
Author

eyalch commented Jan 7, 2019

If I understood correctly, you'd like to know since which Node/npm version npx was available.

It looks like it was added in Node v8.2.0 & npm v5.3.0: https://nodejs.org/en/blog/release/v8.2.0/

@g-plane
Copy link
Member

g-plane commented Jan 9, 2019

Please note that using npx will install ESLint globally. However, running ESLint globally may be different from running it locally, which should be documented.

@platinumazure
Copy link
Member

@g-plane

Please note that using npx will install ESLint globally. However, running ESLint globally may be different from running it locally, which should be documented.

I think that's only the case if the package (in this case ESLint) hasn't already been locally installed? But agreed, this is an important caveat (and now I'm beginning to see why it might not be worth mentioning npx in the documentation).

@eyal0803 Any chance you could add (while keeping things brief) the caveats discussed so far? Minimum Node and npm version, and the fact that npx it should only be run after local installation as a devDependency? Thanks!

Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is moving in the right direction! Just 1-2 more tweaks needed.


```
$ npx eslint
```

**Note:** If wasn't manually installed (via `npm`), `npx` will install `eslint` to a temporary directory and execute it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: "If ESLint wasn't manually installed"

@g-plane Can you comment on the accuracy of "a temporary directory"? Is it really a temporary directory or does npx install to the global installation location? Thanks!

@g-plane
Copy link
Member

g-plane commented Jan 9, 2019

@platinumazure You're right. ESLint will work well if it's installed locally.

Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but would like more eyes on this before merging. Thanks for contributing!

Copy link
Member

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nzakas nzakas merged commit 7c0bf2c into eslint:master Jan 16, 2019
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jul 16, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jul 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants