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

fix: consistent rule doc notices and sections #1226

Merged
merged 26 commits into from
Sep 10, 2022

Conversation

bmish
Copy link
Contributor

@bmish bmish commented Aug 28, 2022

This PR adds the following standardized rule doc notices:

  • 💼 This rule is enabled in the following configs: all, recommended.
  • 🔧 This rule is automatically fixable using the --fix flag on the command line.
  • 💡 This rule provides suggestions that can be applied manually.
  • ❌ This rule is deprecated.

This improves usability by enabling users to clearly and easily find out if a rule is applying to their codebase given their configuration as well as other key rule details like the availability of auto-fixers or auto-suggestions that can help with addressing violations.

All these notices are tested to ensure they are in the correct position and not forgotten. yarn tools:regenerate-docs will also automatically add or update these notices in each rule doc (some inspiration taken from eslint-plugin-unicorn which has a similar command).

It's common for ESLint and many popular ESLint plugins to include notices like these. Some examples (note: I am a maintainer on some of these plugins and have been working to standardize rule documentation):

I also added tests about a few other aspects of rule docs:

  • The title matches the rule description
  • Has a Rule details section
  • Has an Options section only if the rule has options and mentions any named options

@bmish bmish force-pushed the docs-consistent-notices-sections branch 2 times, most recently from 724fb0f to bf22c94 Compare August 28, 2022 22:33
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.

I'll give this a more detailed review later but we've already got a tool for updating our docs which runs in CI so it already covers at least some of what you've added tests for, and automatically does the fixing so want to favor that - would you be interested in expanding that to generate what you've manually added to these docs?

I love the idea of standardizing rule documentation but am wondering if it would be best to try and submit some sort of loose RFC/template so that we could see all the different parts + make it easy to write a tool to help generate them.

@MichaelDeBoey this feels like the sort of thing that would tie in fantastically with the new ESLint community stuff that got landed, so interested in if you have thoughts.

@bmish
Copy link
Contributor Author

bmish commented Aug 28, 2022

@G-Rath thanks for your initial thoughts.

  1. Yes, I can update the rule doc generator to generate these notices as well. That will take some refactoring to make the generator a bit more sophisticated, so I'll work on that over the next week. If you like the updates to the rule docs, it could be worth be merging this first to avoid potential merge conflicts, and then I could create a separate dedicated follow-up PR to focus only on switching to automatic notice generation.
  2. I'm definitely a fan of standardizing the rule documentation as I called out in the PR description. But I'm not sure an ESLint RFC would be appropriate as the ESLint team isn't really in the business of enforcing a documentation standard for plugins. However, we do have generator-eslint, and I like the idea of updating its rule doc template to include notices, so I will plan to do that.

src/__tests__/rules.test.ts Outdated Show resolved Hide resolved
@bmish bmish force-pushed the docs-consistent-notices-sections branch from bf22c94 to 8bfb1de Compare August 29, 2022 15:18
@bmish bmish force-pushed the docs-consistent-notices-sections branch 2 times, most recently from 5a8c5c0 to d1f6e55 Compare September 2, 2022 19:05
@bmish
Copy link
Contributor Author

bmish commented Sep 2, 2022

@G-Rath good news, I went ahead and updated the yarn tools:regenerate-docs command to automatically add or update the rule notices in each rule doc.

@bmish bmish force-pushed the docs-consistent-notices-sections branch from d1f6e55 to 6727301 Compare September 2, 2022 19:22
README.md Show resolved Hide resolved
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.

Looks like a good start - I've put some comments around on how I'd like the output to change; I'll hold off reviewing the tools/ stuff in detail until we've gotten the actual output sorted because that'll impact the actual implementation :)

docs/rules/consistent-test-it.md Outdated Show resolved Hide resolved
docs/rules/consistent-test-it.md Outdated Show resolved Hide resolved
docs/rules/expect-expect.md Outdated Show resolved Hide resolved
docs/rules/no-done-callback.md Outdated Show resolved Hide resolved
docs/rules/no-if.md Outdated Show resolved Hide resolved
src/__tests__/rules.test.ts Outdated Show resolved Hide resolved
src/__tests__/rules.test.ts Outdated Show resolved Hide resolved
tools/regenerate-docs.ts Outdated Show resolved Hide resolved
@G-Rath
Copy link
Collaborator

G-Rath commented Sep 2, 2022

I'm not sure an ESLint RFC would be appropriate as the ESLint team isn't really in the business of enforcing a documentation standard for plugins

No but they have rules themselves that have documentation, and this is also exactly why @eslint-community has been created. I don't think it needs to be a strictly large or full-blown RFC but I do like the idea of something like this going through the RFC process in terms of visibility and discussion as I think it could help unify the community and especially help people that are looking to create their own plugins (I think especially for non-native English speakers, having a template like that would make it a lot easier).

cc @nzakas cause you're probably the best person to give a steer.

@SimenB
Copy link
Member

SimenB commented Sep 2, 2022

This is awesome @bmish, thanks for taking the time! Is the plan to upstream this into some shared eslint tool?

@bmish
Copy link
Contributor Author

bmish commented Sep 5, 2022

I'm not sure an ESLint RFC would be appropriate as the ESLint team isn't really in the business of enforcing a documentation standard for plugins

No but they have rules themselves that have documentation, and this is also exactly why @eslint-community has been created. I don't think it needs to be a strictly large or full-blown RFC but I do like the idea of something like this going through the RFC process in terms of visibility and discussion as I think it could help unify the community and especially help people that are looking to create their own plugins (I think especially for non-native English speakers, having a template like that would make it a lot easier).

This is awesome @bmish, thanks for taking the time! Is the plan to upstream this into some shared eslint tool?

Here's a summary of my plans to standardize ESLint plugin documentation:

  1. Submitting PRs like this one to ensure the top ESLint plugins conform to basic documentation standards (as mentioned in the PR description, I've already completed this for many of the top plugins). The documentation of ESLint and its top plugins will naturally serve as examples for other plugin authors to follow.
  2. Updating the documentation templates in generator-eslint which can be used for generating a new plugin or rule.
  3. Exploring creating a lint rule(s) in eslint-plugin-eslint-plugin for enforcing documentation consistency (title, notices, etc), as most ESLint plugins already use this plugin and would thus easily pull in the new checks. We could also use a lint rule autofixer to generate the auto-generated parts of the rule doc (title, notices).
  4. I'm open to working on some ESLint community recommendations around this, perhaps to point people to the generator-eslint templates or examples of high-quality / standardized documentation.

@bmish bmish requested review from G-Rath and SimenB and removed request for G-Rath and SimenB September 5, 2022 19:17
@bmish bmish requested a review from G-Rath September 6, 2022 01:00
docs/rules/consistent-test-it.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
docs/rules/consistent-test-it.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@G-Rath
Copy link
Collaborator

G-Rath commented Sep 9, 2022

@bmish I've just landed #1239 which updates the tsconfig.json to check tools - could you rebase to include that, as typecheck is failing now.

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.

This is looking awesome! If it's ok with you, I'm keen to toy with this a bit once its landed to see if it can be expanded into a self-standing tool.

We're just got a few type related things to resolve before landing this :)

tools/regenerate-docs.ts Outdated Show resolved Hide resolved
tools/rule-notices.ts Outdated Show resolved Hide resolved
tools/rule-notices.ts Outdated Show resolved Hide resolved
@SimenB SimenB changed the title Consistent rule doc notices and sections fix: consistent rule doc notices and sections Sep 10, 2022
@SimenB SimenB merged commit 2580563 into jest-community:main Sep 10, 2022
@SimenB
Copy link
Member

SimenB commented Sep 10, 2022

Thanks @bmish, this is awesome!

github-actions bot pushed a commit that referenced this pull request Sep 10, 2022
## [27.0.4](v27.0.3...v27.0.4) (2022-09-10)

### Bug Fixes

* consistent rule doc notices and sections ([#1226](#1226)) ([2580563](2580563))
@github-actions
Copy link

🎉 This PR is included in version 27.0.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@bmish
Copy link
Contributor Author

bmish commented Sep 10, 2022

@G-Rath @SimenB thanks a lot for helping to get this in! As part of my goal to standardize ESLint plugin documentation, I agree we should work to share this tool, and I'd be interested to collaborate on that.

If we went with the CLI command here, it could possibly make sense for it to live inside generator-eslint, although that tool currently only has Yeoman generators in it.

I also plan to experiment with whether I could create a lint rule in eslint-plugin-eslint-plugin for enforcing documentation consistency (title, notices, options, etc), as most ESLint plugins already use this plugin and would thus easily pull in the new checks. We could also use a lint rule autofixer to generate the auto-generated parts of the rule doc (title, notices).

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.

5 participants