-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Conversation
`npx` is a shorter alternative to navigating to the executable found in `node_modules/.bin`
There was a problem hiding this 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!
If I understood correctly, you'd like to know since which Node/npm version It looks like it was added in Node v8.2.0 & npm v5.3.0: https://nodejs.org/en/blog/release/v8.2.0/ |
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! |
There was a problem hiding this 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.
docs/user-guide/getting-started.md
Outdated
|
||
``` | ||
$ npx eslint | ||
``` | ||
|
||
**Note:** If wasn't manually installed (via `npm`), `npx` will install `eslint` to a temporary directory and execute it. |
There was a problem hiding this comment.
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!
@platinumazure You're right. ESLint will work well if it's installed locally. |
There was a problem hiding this 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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
npx
is a shorter alternative to navigating to the executable found innode_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
withnpx
.Is there anything you'd like reviewers to focus on?