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

Clarify Linter vs ESLint in node.js api docs #14953

Closed
Brian-Bartels opened this issue Aug 21, 2021 · 12 comments · Fixed by #14995
Closed

Clarify Linter vs ESLint in node.js api docs #14953

Brian-Bartels opened this issue Aug 21, 2021 · 12 comments · Fixed by #14995
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 core Relates to ESLint's core APIs and features documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before
Projects

Comments

@Brian-Bartels
Copy link
Contributor

Brian-Bartels commented Aug 21, 2021

The version of ESLint you are using.
latest

The problem you want to solve.
improve clarity in docs here https://eslint.org/docs/developer-guide/nodejs-api#-eslintlintfilespatterns

Your take on the correct solution to problem.
Just add a few sentences clarify the differences between Linter and ESLint objects as they have a lot of overlap

Are you willing to submit a pull request to implement this change?
Yes, but I feel like someone with more domain knowledge would be more successful!

@Brian-Bartels Brian-Bartels added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Aug 21, 2021
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Aug 21, 2021
@nzakas nzakas changed the title Nit: Clarify Linter vs ESLint in node.js api docs Clarify Linter vs ESLint in node.js api docs Aug 21, 2021
@nzakas
Copy link
Member

nzakas commented Aug 21, 2021

Can you explain what you think is missing or should be clarified?

@nzakas nzakas added documentation Relates to ESLint's documentation and removed enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Aug 21, 2021
@nzakas nzakas moved this from Needs Triage to Triaging in Triage Aug 21, 2021
@Brian-Bartels
Copy link
Contributor Author

Brian-Bartels commented Aug 21, 2021

A bit from each blurb at the tops of the classes could be bubbled up to the top. Also perhaps a sentence on why having access to the file system is useful. For example. I needed some additional ts configurations that I didn't realize. I was trying to use linter. But when I switched to eslint it worked perfectly. Even just a disclaimer at the top "these classes appear similar but have important differences" let me know what you think

@nzakas
Copy link
Member

nzakas commented Aug 24, 2021

We have this blurb under the ESLint class header:

The ESLint class is the primary class to use in Node.js applications.

This class depends on the Node.js fs module and the file system, so you cannot use it in browsers. If you want to lint code on browsers, use the Linter class instead.

Do you think changes should be made there to clarify things?

@Brian-Bartels
Copy link
Contributor Author

Brian-Bartels commented Aug 24, 2021

I suppose that's exactly my point. Unless you read the header for ESLint, and you just read the header for Linter. You'd miss the fact that they are similar

@nzakas
Copy link
Member

nzakas commented Aug 25, 2021

Here’s the description for Linter:

The Linter object does the actual evaluation of the JavaScript code. It doesn't do any filesystem operations, it simply parses and reports on the code. In particular, the Linter object does not process configuration objects or files. The Linter is a constructor, and you can create a new instance by passing in the options you want to use.

What would you change? Would adding a sentence like, “Unless you are working in the browser, you probably want to use the ESLint class instead,” address your concern?

@Brian-Bartels
Copy link
Contributor Author

Brian-Bartels commented Aug 25, 2021

I do think that would help

@nzakas
Copy link
Member

nzakas commented Aug 26, 2021

Cool, that’s feasible. Do you want to submit a pull request for that?

@nzakas nzakas added the accepted There is consensus among the team that this change meets the criteria for inclusion label Aug 26, 2021
@nzakas nzakas moved this from Triaging to Ready to Implement in Triage Aug 26, 2021
@nzakas nzakas added the good first issue Good for people who haven't worked on ESLint before label Aug 26, 2021
@Amoghtech
Copy link

Amoghtech commented Aug 26, 2021

Is this issue open for now?

@Brian-Bartels
Copy link
Contributor Author

Brian-Bartels commented Aug 26, 2021

I'm on vacation away from a computer until Monday. But I can make a quick PR then

@nzakas
Copy link
Member

nzakas commented Aug 27, 2021

Perfect, no hurry.

@Brian-Bartels
Copy link
Contributor Author

Brian-Bartels commented Aug 30, 2021

Well I think I messed up a few things about the title. But I just signed the CLA and hopefully fixed the title #14995

@nzakas nzakas linked a pull request Aug 31, 2021 that will close this issue
1 task
@nzakas
Copy link
Member

nzakas commented Aug 31, 2021

Looks good

Triage automation moved this from Ready to Implement to Complete Aug 31, 2021
nzakas added a commit that referenced this issue Aug 31, 2021
…4995)

* Improve clarity on the usage of the Linter class

* Fix MD issue

* Add line break for readability

* Update docs/developer-guide/nodejs-api.md

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Feb 28, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 28, 2022
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 core Relates to ESLint's core APIs and features documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before
Projects
Triage
Complete
Development

Successfully merging a pull request may close this issue.

3 participants