Docs: update architecture page (fixes #9337) #9345
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@VictorHom, thanks for your PR! By analyzing the history of the files in this pull request, we identified @not-an-aardvark, @nzakas and @alberto to be potential reviewers. |
|
LGTM |
|
LGTM |
platinumazure
requested changes
Sep 23, 2017
docs/developer-guide/architecture.md
Outdated
| @@ -4,7 +4,10 @@ At a high level, there are a few key parts to ESLint: | |||
|
|
|||
| * `bin/eslint.js` - this is the file that actually gets executed with the command line utility. It's a dumb wrapper that does nothing more than bootstrap ESLint, passing the command line arguments to `cli`. This is intentionally small so as not to require heavy testing. | |||
| * `lib/cli.js` - this is the heart of the ESLint CLI. It takes an array of arguments and then uses `eslint` to execute the commands. By keeping this as a separate utility, it allows others to effectively call ESLint from within another Node.js program as if it were done on the command line. The main call is `cli.execute()`. This is also the part that does all the file reading, directory traversing, input, and output. | |||
| * `lib/eslint.js` - this is the core `eslint` object that does code verifying based on configuration options. This file does no file I/O and does not interact with the `console` at all. For other Node.js programs that have JavaScript text to verify, they would be able to use this interface directly. | |||
| * `lib/linter.js` - this is the core `eslint` object that does code verifying based on configuration options by exporting a class Linter. This file does no file I/O and does not interact with the `console` at all. For other Node.js programs that have JavaScript text to verify, they would be able to use this interface directly. | |||
There was a problem hiding this comment.
Linter is now a class, so perhaps you might wish to say something like: "this is the core Linter class that does code verifying..."
docs/developer-guide/architecture.md
Outdated
| * `lib/eslint.js` - this is the core `eslint` object that does code verifying based on configuration options. This file does no file I/O and does not interact with the `console` at all. For other Node.js programs that have JavaScript text to verify, they would be able to use this interface directly. | ||
| * `lib/linter.js` - this is the core `eslint` object that does code verifying based on configuration options by exporting a class Linter. This file does no file I/O and does not interact with the `console` at all. For other Node.js programs that have JavaScript text to verify, they would be able to use this interface directly. | ||
| * `lib/api.js` - this exposes an object that contains Linter, CLIEngine, RuleTester, and SourceCode. | ||
| * `lib/testers/rule-tester.js` - this is a wrapper around mocha, so that rules can be unit tested. |
There was a problem hiding this comment.
Might be worth noting here that rule-tester can be modified to wrap any testing framework, but that it happens to work with Mocha's global testing methods by default and that its interface was modeled after Mocha. (Not sure how best to phrase it, though.)
|
LGTM |
kaicataldo
reviewed
Sep 24, 2017
docs/developer-guide/architecture.md
Outdated
| @@ -4,7 +4,10 @@ At a high level, there are a few key parts to ESLint: | |||
|
|
|||
| * `bin/eslint.js` - this is the file that actually gets executed with the command line utility. It's a dumb wrapper that does nothing more than bootstrap ESLint, passing the command line arguments to `cli`. This is intentionally small so as not to require heavy testing. | |||
| * `lib/cli.js` - this is the heart of the ESLint CLI. It takes an array of arguments and then uses `eslint` to execute the commands. By keeping this as a separate utility, it allows others to effectively call ESLint from within another Node.js program as if it were done on the command line. The main call is `cli.execute()`. This is also the part that does all the file reading, directory traversing, input, and output. | |||
| * `lib/eslint.js` - this is the core `eslint` object that does code verifying based on configuration options. This file does no file I/O and does not interact with the `console` at all. For other Node.js programs that have JavaScript text to verify, they would be able to use this interface directly. | |||
| * `lib/linter.js` - this is the core Linter class that does code verifying based on configuration options by exporting a class Linter. This file does no file I/O and does not interact with the `console` at all. For other Node.js programs that have JavaScript text to verify, they would be able to use this interface directly. | |||
There was a problem hiding this comment.
"this is the core Linter class...by exporting a class Linter".
Can we make this clearer? I personally think this is the core Linter class that does code verifying based on configuration options. is adequate.
docs/developer-guide/architecture.md
Outdated
| * `lib/eslint.js` - this is the core `eslint` object that does code verifying based on configuration options. This file does no file I/O and does not interact with the `console` at all. For other Node.js programs that have JavaScript text to verify, they would be able to use this interface directly. | ||
| * `lib/linter.js` - this is the core Linter class that does code verifying based on configuration options by exporting a class Linter. This file does no file I/O and does not interact with the `console` at all. For other Node.js programs that have JavaScript text to verify, they would be able to use this interface directly. | ||
| * `lib/api.js` - this exposes an object that contains Linter, CLIEngine, RuleTester, and SourceCode. | ||
| * `lib/testers/rule-tester.js` - this is a wrapper around Mocha, so that rules can be unit tested. The RuleTester interface was modeled after Mocha and works with the Mocha's global testing methods. RuleTester can also be modified to work with other testing frameworks. |
There was a problem hiding this comment.
"this is a wrapper around Mocha, so that rules can be unit tested."
Can we describe why this class is useful?
"was modeled after Mocha and works with the Mocha's global testing methods"
Extra the.
|
LGTM |
ilyavolodin
approved these changes
Sep 27, 2017
kaicataldo
suggested changes
Sep 27, 2017
docs/developer-guide/architecture.md
Outdated
| @@ -4,7 +4,10 @@ At a high level, there are a few key parts to ESLint: | |||
|
|
|||
| * `bin/eslint.js` - this is the file that actually gets executed with the command line utility. It's a dumb wrapper that does nothing more than bootstrap ESLint, passing the command line arguments to `cli`. This is intentionally small so as not to require heavy testing. | |||
| * `lib/cli.js` - this is the heart of the ESLint CLI. It takes an array of arguments and then uses `eslint` to execute the commands. By keeping this as a separate utility, it allows others to effectively call ESLint from within another Node.js program as if it were done on the command line. The main call is `cli.execute()`. This is also the part that does all the file reading, directory traversing, input, and output. | |||
| * `lib/eslint.js` - this is the core `eslint` object that does code verifying based on configuration options. This file does no file I/O and does not interact with the `console` at all. For other Node.js programs that have JavaScript text to verify, they would be able to use this interface directly. | |||
| * `lib/linter.js` - this is the core Linter class that does code verifying based on configuration options by exporting a class Linter. | |||
There was a problem hiding this comment.
My apologies, I should have been clearer in my comment. The part that I think is confusing is the part that says "this is the core Linter class that does things by exporting a class Linter.". I think the rest of the information is useful! Here's what I was trying to suggest (sorry it wasn't clearer!) in my last comment:
* `lib/linter.js` - this is the core Linter class that does code verifying based on configuration options. This file does no file I/O and does not interact with the `console` at all. For other Node.js programs that have JavaScript text to verify, they would be able to use this interface directly.
|
LGTM |
kaicataldo
approved these changes
Sep 28, 2017
eslint-deprecated
bot
added
the
archived due to age
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of this pull request? (put an "X" next to item)
update the architecture page on https://eslint.org/docs/developer-guide/architecture
[X] Documentation update
Is there anything you'd like reviewers to focus on?
I haven't worked with all of those files before that are mentioned in the doc update, so if there's anything reviewers think would be important to add, please let me know. Happy to update.