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

Enhance "all" configuration and update documentation #578

Merged
merged 7 commits into from
Dec 1, 2024

Conversation

y-hsgw
Copy link
Contributor

@y-hsgw y-hsgw commented Nov 30, 2024

Resolves: #577

  • Enabled all available rules in the all configuration.
  • Added a new All section in the README to document the all configuration.
  • Removed references to "all configuration." from the Rules section in the README and the documentation for individual rules, as this information is now consolidated in the new All section.
    • These references were removed because the new All section now consolidates this information, making the documentation more organized and easier to follow.

@@ -1,6 +1,5 @@
# Require .spec test file pattern (`vitest/consistent-test-filename`)

⚠️ This rule _warns_ in the 🌐 `all` config.
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 any specific reason why this was removed? It's generated by a tool we use to keep documentation updated. Did you removed it manually?

Copy link
Contributor Author

@y-hsgw y-hsgw Dec 1, 2024

Choose a reason for hiding this comment

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

@veritem

Is there any specific reason why this was removed?

Since the information is already documented in the All section, I determined that it is unnecessary to include it in each individual rule's documentation.

It's generated by a tool we use to keep documentation updated. Did you removed it manually?

Oh, I see... I didn't realize that and ended up removing it manually. 🥲
Are you referring to the script below?

"update:eslint-docs": "pnpm build && eslint-doc-generator",

Copy link
Member

Choose a reason for hiding this comment

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

If you don't mind, you can get them back and I'll go a head and merge your PR

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 reverted it in commit 62cd790.
Should we add the note "This rule warns in the 🌐 all config." to the documentation for the rules that were enabled this time (e.g., expect-expect, no-commented-out-tests, etc.)?

Copy link
Member

Choose a reason for hiding this comment

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

uhmm, it's supposed to be done by eslint-doc-generator automatically but it hasn't added support for flat config.

Should we add the note "This rule warns in the 🌐 all config." to the documentation for the rules that were enabled this time

yes!

Copy link
Contributor Author

@y-hsgw y-hsgw Dec 1, 2024

Choose a reason for hiding this comment

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

I added the notes in ce2114a.

Copy link
Member

Choose a reason for hiding this comment

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

One last thing, did you run this command update:eslint-docs? I think some things on readme have been modified which shouldn't be modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

did you run this command update:eslint-docs?

No, I haven't run it.

I think some things on readme have been modified which shouldn't be modified.

Which part are you referring to? The Rules section?

Copy link
Member

@veritem veritem Dec 1, 2024

Choose a reason for hiding this comment

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

Which part are you referring to? The Rules section?

Yes, the rules section should remain as it is

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 fixed in 488a269.

Copy link
Member

@veritem veritem left a comment

Choose a reason for hiding this comment

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

Thank you!

@veritem veritem merged commit ec2366e into vitest-dev:main Dec 1, 2024
@y-hsgw y-hsgw deleted the update-all-configuration branch December 1, 2024 05:25
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.

Enhance "all" configuration and update documentation
2 participants