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: Configuring Plugins page intro, page tweaks, and rename #16534

Merged
merged 11 commits into from Dec 2, 2022

Conversation

bpmutter
Copy link
Contributor

@bpmutter bpmutter commented Nov 11, 2022

Prerequisites checklist

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

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

What changes did you make? (Give an overview)

  • renamed 'Configuring Plugins' page to 'Configuring Plugins & Parsers' b/c plugins separate from parsers
    • note: didn't change page name b/c didn't want to break the existing URL path before we have redirects set up
  • added intro section to page
  • small copy edits throughout page
  • move section order to make clear that processors are part of plugins, whereas parsers are separate.
  • also moved up the docs on configuring plugins b/c that is the more commonly used feature.

Part of #16473

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

@eslint-github-bot eslint-github-bot bot added triage An ESLint team member will look at this issue soon documentation Relates to ESLint's documentation labels Nov 11, 2022
@netlify
Copy link

netlify bot commented Nov 11, 2022

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 20286ce
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/63854da1f71c450008022828
😎 Deploy Preview https://deploy-preview-16534--docs-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@bpmutter bpmutter marked this pull request as ready for review November 13, 2022 19:04
@snitin315 snitin315 added 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 Nov 15, 2022
Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

Looks good. A few suggestions:

docs/src/user-guide/configuring/plugins.md Outdated Show resolved Hide resolved
docs/src/user-guide/configuring/plugins.md Outdated Show resolved Hide resolved
docs/src/user-guide/configuring/plugins.md Show resolved Hide resolved
Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

Copy link
Member

@amareshsm amareshsm left a comment

Choose a reason for hiding this comment

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

LGTM

* [Configuring Plugins](./plugins#configuring-plugins)
* [Specifying Processor](./plugins#specifying-processor)
* [Configuring Parsers](./plugins#configuring-parsers)
Copy link
Member

Choose a reason for hiding this comment

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

I believe this should be singular to avoid breaking the link and for consistency.

Suggested change
* [Configuring Parsers](./plugins#configuring-parsers)
* [Configuring Parser](./plugins#configuring-parser)

Copy link
Contributor Author

@bpmutter bpmutter Nov 23, 2022

Choose a reason for hiding this comment

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

changed all to be plural, consistent w other similar links on the page.

also updated the #anchor-links to match updated page anchors that don't use "-ing" form (see this comment for context)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. I ❤️ consistency. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

docs/src/user-guide/configuring/plugins.md Show resolved Hide resolved

ESLint checks the file path of named code blocks then ignores those if any `overrides` entry didn't match the file path. Be sure to add an `overrides` entry if you want to lint named code blocks other than `*.js`.

## Configuring Parsers
Copy link
Member

Choose a reason for hiding this comment

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

Keeping everything singular

Suggested change
## Configuring Parsers
## Configuring Parser

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made singular and removed the "-ing" form on all section headers on the page.

in future PR, should update all section headings across entire docs set to be not "-ing" form of word to align w the pages names in the new IA.

@bpmutter bpmutter requested a review from nzakas November 23, 2022 19:49
@bpmutter
Copy link
Contributor Author

@nzakas readded the changes per your suggestions. ready for re-review. (not sure why didn't get added before, presumably some form of PEBCAK.)

bpmutter and others added 2 commits November 28, 2022 19:05
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

* [@babel/eslint-parser](https://www.npmjs.com/package/@babel/eslint-parser) - A wrapper around the [Babel](https://babeljs.io) parser that makes it compatible with ESLint.
* [@typescript-eslint/parser](https://www.npmjs.com/package/@typescript-eslint/parser) - A parser that converts TypeScript into an ESTree-compatible form so it can be used in ESLint.

Note that when using a custom parser, the `parserOptions` configuration property is still required for ESLint to work properly with features not in ECMAScript 5 by default. Parsers are all passed `parserOptions` and may or may not use them to determine which features to enable.
Copy link
Member

Choose a reason for hiding this comment

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

Parsers are all passed `parserOptions` and may or may not use them to determine which features to enable.

I feel this part to be quite confusing 😅

Copy link
Member

Choose a reason for hiding this comment

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

I think this wording is fine and shouldn't hold up this PR. We can continue tweaking later.

* [Specifying Parser](./plugins#specifying-parser)
* [Specifying Processor](./plugins#specifying-processor)
* [Configuring Plugins](./plugins#configuring-plugins)
* [Configuring Plugins](./plugins#configure-plugins)
Copy link
Member

Choose a reason for hiding this comment

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

We still have this inconsistency of "Configuring Plugins" in the link but then it links to "Configure Plugins". The same for the following three links. Can we make these consistent?

Copy link
Contributor Author

@bpmutter bpmutter Nov 30, 2022

Choose a reason for hiding this comment

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

i kept these as they are to match the other heading links on this page, which are all in "-ing" form. was going to change em all as part of #16578

can do these ones now if that's your preference

Copy link
Member

Choose a reason for hiding this comment

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

Ah okay. As long as we're tracking that task, we don't need to do them as part of this PR. 👍

@nzakas nzakas merged commit 0311d81 into eslint:main Dec 2, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Dec 16, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.28.0` -> `8.29.0`](https://renovatebot.com/diffs/npm/eslint/8.28.0/8.29.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.29.0`](https://github.com/eslint/eslint/releases/tag/v8.29.0)

[Compare Source](eslint/eslint@v8.28.0...v8.29.0)

#### Features

-   [`49a07c5`](eslint/eslint@49a07c5) feat: add `allowParensAfterCommentPattern` option to no-extra-parens ([#&#8203;16561](eslint/eslint#16561)) (Nitin Kumar)
-   [`e6a865d`](eslint/eslint@e6a865d) feat: `prefer-named-capture-group` add suggestions ([#&#8203;16544](eslint/eslint#16544)) (Josh Goldberg)
-   [`a91332b`](eslint/eslint@a91332b) feat: In no-invalid-regexp validate flags also for non-literal patterns ([#&#8203;16583](eslint/eslint#16583)) (trosos)

#### Documentation

-   [`0311d81`](eslint/eslint@0311d81) docs: Configuring Plugins page intro, page tweaks, and rename ([#&#8203;16534](eslint/eslint#16534)) (Ben Perlmutter)
-   [`57089b1`](eslint/eslint@57089b1) docs: add a property assignment example for camelcase rule ([#&#8203;16605](eslint/eslint#16605)) (Milos Djermanovic)
-   [`b6ab030`](eslint/eslint@b6ab030) docs: add docs codeowners ([#&#8203;16601](eslint/eslint#16601)) (Strek)
-   [`6380c87`](eslint/eslint@6380c87) docs: fix sitemap and feed ([#&#8203;16592](eslint/eslint#16592)) (Milos Djermanovic)
-   [`ade621d`](eslint/eslint@ade621d) docs: perf debounce the search query ([#&#8203;16586](eslint/eslint#16586)) (Shanmughapriyan S)
-   [`fbcf3ab`](eslint/eslint@fbcf3ab) docs: fix searchbar clear button ([#&#8203;16585](eslint/eslint#16585)) (Shanmughapriyan S)
-   [`f894035`](eslint/eslint@f894035) docs: HTTPS link to yeoman.io ([#&#8203;16582](eslint/eslint#16582)) (Christian Oliff)
-   [`de12b26`](eslint/eslint@de12b26) docs: Update configuration file pages ([#&#8203;16509](eslint/eslint#16509)) (Ben Perlmutter)
-   [`1ae9f20`](eslint/eslint@1ae9f20) docs: update correct code examples for `no-extra-parens` rule ([#&#8203;16560](eslint/eslint#16560)) (Nitin Kumar)

#### Chores

-   [`7628403`](eslint/eslint@7628403) chore: add discord channel link ([#&#8203;16590](eslint/eslint#16590)) (Amaresh  S M)
-   [`f5808cb`](eslint/eslint@f5808cb) chore: fix rule doc headers check ([#&#8203;16564](eslint/eslint#16564)) (Milos Djermanovic)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC40Ny4xIiwidXBkYXRlZEluVmVyIjoiMzQuNTQuMiJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1667
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jun 1, 2023
@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 Jun 1, 2023
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