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

Automate docs with eslint-doc-generator #901

Merged
merged 1 commit into from
Oct 30, 2022

Conversation

bmish
Copy link
Contributor

@bmish bmish commented Oct 27, 2022

I built this CLI tool eslint-doc-generator for automating the generation of the README rules list and rule doc title/notices for ESLint plugins. It follows common documentation conventions from this and other top ESLint plugins and will help us standardize documentation across ESLint plugins (and generally improve the usability of our custom rules through better documentation and streamline the process of adding new rules).

This builds on some of the documentation work done by @uncommon-type in #837. eslint-doc-generator is more robust compared to the previous documentation generator script which I have now removed. It has additional functionality (for example, the rules list legend is auto-generated, and the rules list will show additional columns of information when applicable) and 100% test coverage. An equivalent PR to eslint-plugin-react was also merged recently: jsx-eslint/eslint-plugin-react#3469.

Closes #903.

src/rules/accessible-emoji.js Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 27, 2022

Codecov Report

Merging #901 (9dd3ce1) into main (0bdf95b) will not change coverage.
The diff coverage is n/a.

❗ Current head 9dd3ce1 differs from pull request most recent head 6d7a857. Consider uploading reports for the commit 6d7a857 to get more accurate results

@@           Coverage Diff           @@
##             main     #901   +/-   ##
=======================================
  Coverage   99.29%   99.29%           
=======================================
  Files         103      103           
  Lines        1571     1571           
  Branches      514      514           
=======================================
  Hits         1560     1560           
  Misses         11       11           
Impacted Files Coverage Δ
...s/no-interactive-element-to-noninteractive-role.js 100.00% <ø> (ø)
...rc/rules/no-noninteractive-element-interactions.js 100.00% <ø> (ø)
...s/no-noninteractive-element-to-interactive-role.js 100.00% <ø> (ø)
src/rules/no-noninteractive-tabindex.js 97.36% <ø> (ø)
src/rules/no-static-element-interactions.js 100.00% <ø> (ø)
src/rules/scope.js 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@bmish bmish force-pushed the eslint-doc-generator branch 2 times, most recently from c804098 to bfdc5cf Compare October 27, 2022 01:35
package.json Outdated Show resolved Hide resolved
.github/workflows/readme.yml Outdated Show resolved Hide resolved
docs/rules/alt-text.md Outdated Show resolved Hide resolved
docs/rules/label-has-for.md Outdated Show resolved Hide resolved
docs/rules/prefer-tag-over-role.md Outdated Show resolved Hide resolved
src/rules/accessible-emoji.js Outdated Show resolved Hide resolved
| :----------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------------------------------------------------------- | :---- | :-- |
| [accessible-emoji](docs/rules/accessible-emoji.md) | | | ❌ |
| [alt-text](docs/rules/alt-text.md) | Enforce all elements that require alternative text have meaningful information to relay back to end user. | ☑️ 🔒 | |
| [anchor-ambiguous-text](docs/rules/anchor-ambiguous-text.md) | Enforce `<a>` text to not exactly match "click here", "here", "link", or "a link". | | |
Copy link
Member

Choose a reason for hiding this comment

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

is there an easy way to make these links be footnoted, so this could be:

Suggested change
| [anchor-ambiguous-text](docs/rules/anchor-ambiguous-text.md) | Enforce `<a>` text to not exactly match "click here", "here", "link", or "a link". | | |
| [anchor-ambiguous-text] | Enforce `<a>` text to not exactly match "click here", "here", "link", or "a link". | | |

and the raw table would be smaller?

Copy link
Contributor Author

@bmish bmish Oct 28, 2022

Choose a reason for hiding this comment

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

That would make the table markdown more aesthetic, but I'm not sure that matters much since it's generated and managed by the tool.

Now if we expected or wanted to support the rule names being linked to from elsewhere in the readme, I could see an argument for switching to the reusable footnote link style, or perhaps having an option to switch to footnote links, but it would be good to spell out a use case for that first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A downside of switching to the footnote links is doubling the managed markdown height (from the table + the list of rule links).

Copy link
Member

Choose a reason for hiding this comment

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

I feel like that's less of an issue, personally, since it's easier for humans to read the narrower table.

Certainly not a blocker, but I do think it would be an advantage to use footnotes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I'm going to pursue this idea at the moment, but I'll at least track it here: bmish/eslint-doc-generator#189

@ljharb ljharb force-pushed the eslint-doc-generator branch from 03b41b5 to 6d7a857 Compare October 30, 2022 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Repo tooling: add testing to ensure each rule has a corresponding .md
2 participants