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 #243

Merged
merged 1 commit into from Oct 29, 2022

Conversation

bmish
Copy link
Collaborator

@bmish bmish commented Oct 12, 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 #106 #107. 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.

I adjusted CI so that we can run the lint job in only Node 18 (as recommended in #242 (review)) as this tool requires Node >= 14.

@bmish bmish added the docs label Oct 12, 2022
@bmish bmish force-pushed the eslint-doc-generator branch 2 times, most recently from 4688b0a to a5096cd Compare October 19, 2022 02:39
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a5096cd on bmish:eslint-doc-generator into f789574 on platinumazure:master.

@coveralls
Copy link

coveralls commented Oct 19, 2022

Coverage Status

Coverage remained the same at 100.0% when pulling 9be0b57 on bmish:eslint-doc-generator into f789574 on platinumazure:master.

@bmish bmish force-pushed the eslint-doc-generator branch 3 times, most recently from fe1b71d to 25a8e12 Compare October 20, 2022 16:22
@bmish bmish marked this pull request as ready for review October 20, 2022 16:22
@bmish
Copy link
Collaborator Author

bmish commented Oct 20, 2022

@platinumazure I split out the lint job from the test job in CI so that we can run the lint job in only Node 18 (as you recommended in #242 (review)) as this tool requires Node >= 14.

This is ready for review.

@bmish bmish force-pushed the eslint-doc-generator branch 2 times, most recently from 3b36298 to 3d9b936 Compare October 26, 2022 15:06
Copy link
Owner

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

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

Looks great overall! Just not sure about the changes to package scripts. Thanks for this!

package.json Outdated
"preversion": "npm test",
"update": "node ./scripts/update-rules.js",
"report-coverage-html": "nyc report --reporter=html --report-dir build/coverage",
"test": "nyc mocha tests/**/*.js",
Copy link
Owner

Choose a reason for hiding this comment

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

Would it be possible to have npm test still run linting and just configure CI to call unit-test and lint as appropriate?

I feel that most developers are used to having npm test do everything CI would do. And I don't think it would be a good experience for someone to push a branch and only have linting fail at that point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@platinumazure sounds good, I reverted to having npm test run testing and linting.

@bmish bmish force-pushed the eslint-doc-generator branch 2 times, most recently from 7c5b18c to dcaa8b1 Compare October 27, 2022 14:57
@bmish bmish mentioned this pull request Oct 27, 2022
Copy link
Owner

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@platinumazure platinumazure merged commit f9840e7 into platinumazure:master Oct 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants