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: use eslint-doc-generator for rule documentation #1263

Merged
merged 1 commit into from Nov 4, 2022

Conversation

bmish
Copy link
Contributor

@bmish bmish commented Oct 10, 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 is a follow-up to the work I did to add the original notices in #1226 (and thanks @G-Rath and @SimenB for helping to inspire it). eslint-doc-generator is significantly more robust compared to the previous documentation generator script/tests which I have now removed. It has much more 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.

There are some discrepancies between the old docs and the new docs. Most of these are intentional and likely improvements. If you don't like any of the changes, let me know and I can tweak eslint-doc-generator or add a config option to support the desired behavior.

Some differences include:

  • The all config is hidden everywhere as a result of the --ignore-config all option
  • Removed the ### Default configuration section header from some rule docs which was confusing because it was used for rules with no config/options (and causes the options section check to fail)

@G-Rath
Copy link
Collaborator

G-Rath commented Oct 10, 2022

This sounds really cool! I'll have a proper look when I've got some time, but two initial thoughts:

  • would it be possible to support having a different table for the type based rules? the reason I did that originally is because it seemed distracting to have a whole other column for something that only applies for one rule, and that we're probably not going to see any more type-based rules anytime soon (I expect that's probably true for a lot of plugins outside of @typescript-eslint)
  • it looks like the script entry in package.json is pretty large - maybe it would make sense to have a lightweight config json file?

@bmish
Copy link
Contributor Author

bmish commented Oct 10, 2022

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

yay, this is awesome! stoked to see this land when Gareth is happy 🙂

@SimenB
Copy link
Member

SimenB commented Oct 11, 2022

Hmm, weird CI failure needs to be investigated

@bmish
Copy link
Contributor Author

bmish commented Oct 26, 2022

I implemented a feature to separate the type-checking rules list with --split-by meta.docs.requiresTypeChecking and then was able to hide the type-checking column from the lists using the --rule-list-columns option.

Still need to investigate the CI failure.

@bmish bmish force-pushed the eslint-doc-generator branch 2 times, most recently from b5b1a46 to 9209b14 Compare October 28, 2022 17:09
docs/rules/expect-expect.md Outdated Show resolved Hide resolved
@bmish
Copy link
Contributor Author

bmish commented Oct 28, 2022

The CI failure has been fixed by ensuring we build before running the generator.

This PR is ready for review.

Note that I plan to implement this feature eventually but would prefer not to block the PR on it:

@bmish bmish force-pushed the eslint-doc-generator branch 2 times, most recently from 1c69d98 to 977ecd5 Compare October 30, 2022 18:55
@bmish bmish force-pushed the eslint-doc-generator branch 4 times, most recently from 422bac9 to b3b6a70 Compare November 1, 2022 18:34
@bmish
Copy link
Contributor Author

bmish commented Nov 2, 2022

Just a heads up that this PR is still ready. I'm just updating it whenever I publish a new version of the tool.

@G-Rath
Copy link
Collaborator

G-Rath commented Nov 2, 2022

sweet - I'll try and have a look over the weekend; thanks again for your work on this!

Copy link
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

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

Great stuff! I have some mild feelings about if we really need so many columns in the table (i.e. the "Warn" and "fix vs suggest" specifically), but other maintainers don't seem mind and hey this is exactly what I was hoping for in the long run with my regenerate-docs script 😂

package.json Show resolved Hide resolved
@bmish
Copy link
Contributor Author

bmish commented Nov 3, 2022

I have some mild feelings about if we really need so many columns in the table (i.e. the "Warn" and "fix vs suggest" specifically)

The good news is that the columns are configurable. I filed an issue to consider a consolidated column for fixable/suggestions: bmish/eslint-doc-generator#207.

For the configuration columns, I've gone through multiple iterations of it and the current design of a separate column per configured rule severity is by far the most scalable and robust solution I've reached. I didn't find a good way to represent different severities in a single config column.

@G-Rath G-Rath changed the title Automate docs with eslint-doc-generator docs: use eslint-doc-generator for rule documentation Nov 4, 2022
@G-Rath G-Rath merged commit b9faa0f into jest-community:main Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants