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: Update configuration file pages #16509

Merged
merged 8 commits into from Nov 24, 2022

Conversation

bpmutter
Copy link
Contributor

@bpmutter bpmutter commented Nov 6, 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)

Clean up page copy and add small introductions on pages Configuration Files and Configuration Files (New)

Notes:

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

Part of fix for #16473
Resolves #16478

clean up page copy and add small introductions.

Part of fix for eslint#16473
@eslint-github-bot eslint-github-bot bot added documentation Relates to ESLint's documentation triage An ESLint team member will look at this issue soon labels Nov 6, 2022
@netlify
Copy link

netlify bot commented Nov 6, 2022

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit a5e1b9c
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/637e71580d14ed0009d13bd3
😎 Deploy Preview https://deploy-preview-16509--docs-eslint.netlify.app/user-guide/configuring/configuration-files-new
📱 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 draft November 6, 2022 21:14
@bpmutter bpmutter marked this pull request as ready for review November 13, 2022 18:19
@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
@@ -547,7 +549,7 @@ Here, the second configuration object only overrides the severity, so the final

### Configuring shared settings

ESLint supports adding shared settings into configuration files. Plugins use `settings` to specify information that should be shared across all of its rules. You can add a `settings` object to a configuration object and it will be supplied to every rule being executed. This may be useful if you are adding custom rules and want them to have access to the same information. Here's an example:
ESLint supports adding shared settings into configuration files. Plugins use `settings` to specify information that should be shared across all of its rules. When you add a `settings` object to a configuration object, and it is supplied to every rule executed by that configuration object. This may be useful if you are adding custom rules and want them to have access to the same information. Here's an example:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ESLint supports adding shared settings into configuration files. Plugins use `settings` to specify information that should be shared across all of its rules. When you add a `settings` object to a configuration object, and it is supplied to every rule executed by that configuration object. This may be useful if you are adding custom rules and want them to have access to the same information. Here's an example:
ESLint supports adding shared settings into configuration files. Plugins use `settings` to specify the information that should be shared across all of its rules. When you add a `settings` object to a configuration object, and it is supplied to every rule executed by that configuration object. This may be useful if you are adding custom rules and want them to have access to the same information. Here's an example:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change looks good.

but upon re-reading the sentence, i think there's something wrong with the 'across all its rules' phrase. what does 'its' refer to?

@snitin315 or @nzakas, do you know?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it is referring to the Plugin here.

Copy link
Member

Choose a reason for hiding this comment

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

That’s correct. Note: settings are not limited to one plugin. Every plugin receives every setting that was set in the config. It’s a convention that plugins namespace the settings they are interested in to avoid collisions with others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the clarifications. i believe i've addressed this confusion in the latest commit, 53dd21d.

also added the word 'the' before information that started this conversation.

Co-authored-by: Strek <ssharishkumar@gmail.com>
Co-authored-by: Amaresh  S M  <amareshsm13@gmail.com>
@bpmutter
Copy link
Contributor Author

applied suggestions from code review. 1 outstanding question.

@bpmutter
Copy link
Contributor Author

addressed outstanding question. ready for re-review

Co-authored-by: Strek <ssharishkumar@gmail.com>
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.

Lint is failing, can you check?

@mdjermanovic
Copy link
Member

Lint is failing, can you check?

Will be fixed by #16564

@@ -547,7 +549,7 @@ Here, the second configuration object only overrides the severity, so the final

### Configuring shared settings

ESLint supports adding shared settings into configuration files. Plugins use `settings` to specify information that should be shared across all of its rules. You can add a `settings` object to a configuration object and it will be supplied to every rule being executed. This may be useful if you are adding custom rules and want them to have access to the same information. Here's an example:
ESLint supports adding shared settings into configuration files. When you add a `settings` object to a configuration object, it is supplied to every rule executed by that configuration object. By convention, plugins namespace the settings they are interested in to avoid collisions with others. Plugins can use `settings` to specify the information that should be shared across all of their rules. This may be useful if you are adding custom rules and want them to have access to the same information. Here's an example:
Copy link
Member

Choose a reason for hiding this comment

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

"it is supplied to every rule executed by that configuration object" might be misleading. It sounds like the settings object is only supplied to rules: {} from the same configuration object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdjermanovic
what'd be correct, it's applied to all rules in the project?

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 you can just drop “executed by that configuration object”.

Copy link
Member

Choose a reason for hiding this comment

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

It's supplied to every rule when linting a file to which the configuration object applies, but those rules don't have to be enabled by that configuration object.

For example:

module.exports = [
    {
        rules: {
            "my-plugin/my-rule": "error"
        }
    },
    {
        files: ["a.js"],
        settings: {
            data: "something"
        }
    }
];

When linting a.js, my-plugin/my-rule will be supplied with { data: "something" } (as context.settings), although my-plugin/my-rule rule isn't enabled in the same configuration object where settings are defined.

Since this sentence only briefly describes what settings means, it could just say "it is supplied to every rule", as @nzakas suggested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the further clarification, implementing nicholas's suggestion!

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
@bpmutter bpmutter requested review from nzakas and mdjermanovic and removed request for mdjermanovic and nzakas November 23, 2022 19:15
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. Just waiting for @mdjermanovic to review.

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!

@mdjermanovic mdjermanovic merged commit de12b26 into eslint:main Nov 24, 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 May 24, 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 May 24, 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.

Configuring documentation section cleanup
6 participants