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: Formatters page updates #16566

Merged
merged 8 commits into from
Dec 6, 2022

Conversation

bpmutter
Copy link
Contributor

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)

Miscellaneous changes on Formatters page:

  • In the example source section, make fullOfProblems.js and .eslintrc: not headers.
  • For the Example Outputs section, provide an example of the CLI input, and also give a few sentence description of what each of the output is, and why you might want to use it.
  • Other small cosmetic changes and refactors

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

Fixes #16476

@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 22, 2022
@netlify
Copy link

netlify bot commented Nov 22, 2022

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit 7658246
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/638bd5ae7002370007010537

docs/src/user-guide/formatters/index.md Outdated Show resolved Hide resolved
docs/src/user-guide/formatters/index.md Outdated Show resolved Hide resolved
@@ -42,7 +42,7 @@ function addOne(i) {
};
```

### `.eslintrc`
`.eslintrc`:
Copy link
Member

Choose a reason for hiding this comment

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

While we're at it, let's use a file extension:

Suggested change
`.eslintrc`:
`.eslintrc.json`:

docs/src/user-guide/formatters/index.md Outdated Show resolved Hide resolved
docs/src/user-guide/formatters/index.md Outdated Show resolved Hide resolved
docs/src/user-guide/formatters/index.md Outdated Show resolved Hide resolved
docs/src/user-guide/formatters/index.md Outdated Show resolved Hide resolved
docs/src/user-guide/formatters/index.md Outdated Show resolved Hide resolved
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.

Note: This page is autogenerated. Any changes made here will go live but will be overwrriten the next time the site is generated. The evergreen changes should be made in this file:

https://github.com/eslint/eslint/blob/main/templates/formatter-examples.md.ejs

@bpmutter
Copy link
Contributor Author

@nzakas and @mdjermanovic thank you for the context! i'll try to fill out all the descriptions with this information. will let you know in comments if need any further clarification here.

and regarding:

Note: This page is autogenerated. Any changes made here will go live but will be overwrriten the next time the site is generated. The evergreen changes should be made in this file:

https://github.com/eslint/eslint/blob/main/templates/formatter-examples.md.ejs

at first glance, i understand what's going on with the template conceptually, but am not sure how i'd add the descriptions to it. will dig in a bit, and let you know if any questions.

if i cannot figure out how to add the descriptions to the template, i'll add them to the markdown file, so we can at least get the copy down before later adding it to the template file.

add more information about the built-in formatters

Fixes eslint#16476
@bpmutter bpmutter marked this pull request as ready for review November 27, 2022 21:29
@bpmutter
Copy link
Contributor Author

i've updated the code to include descriptions in the templates by adding a _manifest.js file to the source code, which contains the descriptions.

i've also:

  • updated the template to include descriptions and changes that were previously made to docs/src/user-guide/formatters/index.md.
  • reverted the changes to docs/src/user-guide/formatters/index.md since the page is generated.

generate the output as expected, you can run npm run gensite.

i'm not convinced that using the _manifest.js file as is done here is the best approach for including descriptions from the source code, even if it is effective. i look forward to the maintainer team's feedback on if/how to refactor this.

@mdjermanovic mdjermanovic 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 30, 2022
@@ -0,0 +1,64 @@
/**
Copy link
Member

Choose a reason for hiding this comment

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

Because this is just data, can we just create a JSON file here? I'd call it formatter-data.json and you'd still be able to require() it, but would otherwise be ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sgtm, i like this approach

@bpmutter
Copy link
Contributor Author

@nzakas implemented w the file named formatter-meta.json to align w the file naming convention of rules-meta.json

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. Would like @mdjermanovic to review before merging.

lib/cli-engine/formatters/formatters-meta.json Outdated Show resolved Hide resolved
lib/cli-engine/formatters/formatters-meta.json Outdated Show resolved Hide resolved
templates/formatter-examples.md.ejs Outdated Show resolved Hide resolved
templates/formatter-examples.md.ejs Outdated Show resolved Hide resolved
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
@bpmutter bpmutter requested a review from a team as a code owner December 3, 2022 23:03
@bpmutter
Copy link
Contributor Author

bpmutter commented Dec 6, 2022

@mdjermanovic ready for re-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 dfc7ec1 into eslint:main Dec 6, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Dec 24, 2022
This PR contains the following updates:

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

---

### Release Notes

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

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

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

#### Features

-   [`075ef2c`](eslint/eslint@075ef2c) feat: add suggestion for no-return-await ([#&#8203;16637](eslint/eslint#16637)) (Daniel Bartholomae)
-   [`7190d98`](eslint/eslint@7190d98) feat: update globals ([#&#8203;16654](eslint/eslint#16654)) (Sébastien Règne)

#### Bug Fixes

-   [`1a327aa`](eslint/eslint@1a327aa) fix: Ensure flat config unignores work consistently like eslintrc ([#&#8203;16579](eslint/eslint#16579)) (Nicholas C. Zakas)
-   [`9b8bb72`](eslint/eslint@9b8bb72) fix: autofix recursive functions in no-var ([#&#8203;16611](eslint/eslint#16611)) (Milos Djermanovic)

#### Documentation

-   [`6a8cd94`](eslint/eslint@6a8cd94) docs: Clarify Discord info in issue template config ([#&#8203;16663](eslint/eslint#16663)) (Nicholas C. Zakas)
-   [`ad44344`](eslint/eslint@ad44344) docs: CLI documentation standardization ([#&#8203;16563](eslint/eslint#16563)) (Ben Perlmutter)
-   [`293573e`](eslint/eslint@293573e) docs: fix broken line numbers ([#&#8203;16606](eslint/eslint#16606)) (Sam Chen)
-   [`fa2c64b`](eslint/eslint@fa2c64b) docs: use relative links for internal links ([#&#8203;16631](eslint/eslint#16631)) (Percy Ma)
-   [`75276c9`](eslint/eslint@75276c9) docs: reorder options in no-unused-vars ([#&#8203;16625](eslint/eslint#16625)) (Milos Djermanovic)
-   [`7276fe5`](eslint/eslint@7276fe5) docs: Fix anchor in URL ([#&#8203;16628](eslint/eslint#16628)) (Karl Horky)
-   [`6bef135`](eslint/eslint@6bef135) docs: don't apply layouts to html formatter example ([#&#8203;16591](eslint/eslint#16591)) (Tanuj Kanti)
-   [`dfc7ec1`](eslint/eslint@dfc7ec1) docs: Formatters page updates ([#&#8203;16566](eslint/eslint#16566)) (Ben Perlmutter)
-   [`8ba124c`](eslint/eslint@8ba124c) docs: update the `prefer-const` example ([#&#8203;16607](eslint/eslint#16607)) (Pavel)
-   [`e6cb05a`](eslint/eslint@e6cb05a) docs: fix css leaking ([#&#8203;16603](eslint/eslint#16603)) (Sam Chen)

#### Chores

-   [`f2c4737`](eslint/eslint@f2c4737) chore: upgrade [@&#8203;eslint/eslintrc](https://github.com/eslint/eslintrc)[@&#8203;1](https://github.com/1).4.0 ([#&#8203;16675](eslint/eslint#16675)) (Milos Djermanovic)
-   [`ba74253`](eslint/eslint@ba74253) chore: standardize npm script names per [#&#8203;14827](eslint/eslint#14827) ([#&#8203;16315](eslint/eslint#16315)) (Patrick McElhaney)
-   [`0d9af4c`](eslint/eslint@0d9af4c) ci: fix npm v9 problem with `file:` ([#&#8203;16664](eslint/eslint#16664)) (Milos Djermanovic)
-   [`90c9219`](eslint/eslint@90c9219) refactor: migrate off deprecated function-style rules in all tests ([#&#8203;16618](eslint/eslint#16618)) (Bryan Mishkin)

</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:eyJjcmVhdGVkSW5WZXIiOiIzNC43MC40IiwidXBkYXRlZEluVmVyIjoiMzQuNzAuNCJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1689
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 5, 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 5, 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.

Formatters Page Clean Up & Expansion
3 participants