Skip to content

docs(eslint-plugin): add warning about superfluous rules with typescript #7372

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 20 commits into from
Sep 16, 2023
Merged

docs(eslint-plugin): add warning about superfluous rules with typescript #7372

merged 20 commits into from
Sep 16, 2023

Conversation

Zamiell
Copy link
Contributor

@Zamiell Zamiell commented Jul 28, 2023

PR Checklist

Overview

Adding warning to docs. We probably want to turn this into a template instead, but not sure how to go about that right now.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @Zamiell!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

@netlify
Copy link

netlify bot commented Jul 28, 2023 •
edited
Loading

✅ Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 53d4266
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/65048a5b2b5e320009dd4d24
😎 Deploy Preview https://deploy-preview-7372--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 1 paths audited
Performance: 90 (🔴 down 9 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 98 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@nx-cloud
Copy link

nx-cloud bot commented Jul 28, 2023 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 53d4266. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 20 targets

Sent with 💌 from NxCloud.

JoshuaKGoldberg
JoshuaKGoldberg previously approved these changes Aug 5, 2023
View reviewed changes
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

LGTM (thanks!), but will wait for other input since maybe we'd want to automate this a bit more?

@JoshuaKGoldberg JoshuaKGoldberg added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Aug 5, 2023
@bradzacher bradzacher changed the title Docs: add typescript warning docs(eslint-plugin): add typescript warning Aug 9, 2023
bradzacher
bradzacher previously requested changes Aug 9, 2023
View reviewed changes
Zamiell and others added 2 commits August 9, 2023 10:52
@Zamiell @bradzacher
Update packages/eslint-plugin/docs/rules/no-dupe-class-members.md
91b8263 
Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
@Zamiell @bradzacher
Update packages/eslint-plugin/docs/rules/no-invalid-this.md
c44d1c3 
Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
@Zamiell
Copy link
Contributor Author

Zamiell commented Aug 25, 2023

I haven't heard back from @Josh-Cena in 2 weeks, so I'll disregard his comment and try using a JSX component.

@Zamiell
Copy link
Contributor Author

Zamiell commented Aug 26, 2023

Ok, things are looking good now with an MDX component.

I also performed a short audit of the rules and found that the warning message also needed to be added to the no-redeclare rule.

This PR is now ready for review again.

@Josh-Cena
Copy link
Member

Oh sorry, I didn't realize there are things pending on my feedback. FYI @Zamiell MDX partials also support taking props:

https://docusaurus.io/docs/markdown-features/react#importing-markdown

It should be as easy as taking your component's returned JSX element and converting it to Markdown.

I don't have much objection to the current approach, though.

@Zamiell
Copy link
Contributor Author

Zamiell commented Aug 26, 2023

Looks like the "CI / Website tests (pull_request)" is failing, but I don't think it has anything to do with my PR, correct me if I am wrong.

@Zamiell
Copy link
Contributor Author

Zamiell commented Sep 15, 2023

It's been 3 weeks and I haven't heard anything from anyone on the team, so pinging Josh/Brad.

@Zamiell Zamiell mentioned this pull request Sep 15, 2023
Closed
2 tasks
JoshuaKGoldberg
JoshuaKGoldberg approved these changes Sep 15, 2023
View reviewed changes
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

✨ super, thanks!

I like this usage of mdx 😄

@JoshuaKGoldberg JoshuaKGoldberg dismissed bradzacher’s stale review September 16, 2023 03:39

chatted directly, ✅

@JoshuaKGoldberg JoshuaKGoldberg changed the title docs(eslint-plugin): add typescript warning docs(eslint-plugin): add warning about superfluous rules with typescript Sep 16, 2023
@JoshuaKGoldberg JoshuaKGoldberg merged commit e0cb751 into typescript-eslint:main Sep 16, 2023
@Zamiell Zamiell deleted the docs branch September 16, 2023 03:40
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Josh-Cena Josh-Cena Josh-Cena left review comments

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg approved these changes

@bradzacher bradzacher bradzacher left review comments

Assignees
No one assigned
Labels
1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docs: Document Superfluous Rules with TypeScript
4 participants
@Zamiell @Josh-Cena @JoshuaKGoldberg @bradzacher