Skip to content

feat: remove TSLint #866

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
jakebailey merged 7 commits into from
Dec 14, 2023
Merged

feat: remove TSLint #866

jakebailey merged 7 commits into from
Dec 14, 2023

Conversation

@JoshuaKGoldberg
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg commented Dec 12, 2023 •
edited
Loading

Fixes #648! 🥳🔪

Now that all the old TSLint rules have been switch to ESLint equivalents, there's no more need for TSLint in dtslint. This PR removes the tslint package reference and tooling around it.

Edit: Actually, one more set of steps. See #866 (comment).

JoshuaKGoldberg
JoshuaKGoldberg commented Dec 12, 2023
View reviewed changes
@jakebailey
Copy link
Member

jakebailey commented Dec 12, 2023

Also todo:

  • Remove all tslint stuff from DT proper
  • Remove code that looks from dt-mergebot
  • Maybe, error if tslint.json exists

sandersn
sandersn requested changes Dec 12, 2023
View reviewed changes
@JoshuaKGoldberg JoshuaKGoldberg mentioned this pull request Dec 13, 2023
Merged
@jakebailey
Copy link
Member

jakebailey commented Dec 13, 2023

There's one reference left to tslint in packages/dtslint/README.md.

@jakebailey
Copy link
Member

jakebailey commented Dec 13, 2023

CI still fails from knip, but we're close.

sandersn
sandersn approved these changes Dec 13, 2023
View reviewed changes
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

looks good, although it also needs a matching DT PR to remove tslint.jsons there, and a pnpm changeset entry.

@jakebailey
Copy link
Member

jakebailey commented Dec 13, 2023

I have no idea how this didn't fail the changeset test.

jakebailey
jakebailey reviewed Dec 14, 2023
View reviewed changes
jakebailey
jakebailey reviewed Dec 14, 2023
View reviewed changes
@JoshuaKGoldberg
Copy link
Contributor Author

JoshuaKGoldberg commented Dec 14, 2023

matching DT PR to remove tslint.jsons there

Right, the existing core TSLint rule disables... blurgh. Will work on that now.

@JoshuaKGoldberg
Copy link
Contributor Author

JoshuaKGoldberg commented Dec 14, 2023 •
edited
Loading

I think it'd actually make sense to port the references to core TSLint rules over to ESLint / typescript-eslint (dtslint.json -> tslint.json). Marking as draft until then. Never mind.

@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as draft December 14, 2023 01:52
@jakebailey jakebailey mentioned this pull request Dec 14, 2023
Merged
@sandersn
Copy link
Member

sandersn commented Dec 14, 2023

Wait a minute, dt.json and dtslint.json are empty now. At least the ones in packages/dtslint are.

@sandersn sandersn mentioned this pull request Dec 14, 2023
Merged
@sandersn
Copy link
Member

sandersn commented Dec 14, 2023

I deleted tslint.jsons on DT just now: DefinitelyTyped/DefinitelyTyped#67890

I'm going to heat my office by testing locally while I go make breakfast.

@sandersn
Copy link
Member

sandersn commented Dec 14, 2023

@JoshuaKGoldberg are you talking about the rules that we deleted without moving to eslint? We triaged the rules into good, bad, decent-but-too-much--effort and only kept the good ones.

@jakebailey
Copy link
Member

jakebailey commented Dec 14, 2023

Yeah, once my one comment is fixed, I think this PR is good, so I'm not sure why it's drafted.

@JoshuaKGoldberg
Copy link
Contributor Author

JoshuaKGoldberg commented Dec 14, 2023

OH! I did a silly and mixed up dtslint/tslint.json and dtslint/dtslint.json.

DefinitelyTyped-tools/packages/dtslint/tslint.json

Line 2 in 924e506

"extends": "tslint:latest",

Never mind, apologies for the disruption .🙂

@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review December 14, 2023 18:25
@jakebailey jakebailey merged commit 02c11c3 into microsoft:main Dec 14, 2023
jakebailey
jakebailey reviewed Dec 14, 2023
View reviewed changes
.changeset/cuddly-crabs-tie.md
Comment on lines +2 to +3
"@definitelytyped/header-parser": minor
"@definitelytyped/dtslint": minor
Copy link
Member

@jakebailey jakebailey Dec 14, 2023

Choose a reason for hiding this comment

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

I failed to notice that these said minor. They should have been patch. We are 0.0.x without stability and now we just released partial 0.1.0s :(((((((((((((((

Copy link
Contributor Author

@JoshuaKGoldberg JoshuaKGoldberg Dec 14, 2023

Choose a reason for hiding this comment

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

Oh geez. sorry!

@JoshuaKGoldberg JoshuaKGoldberg deleted the remove-tslint branch December 14, 2023 20:16
@jakebailey jakebailey mentioned this pull request Jan 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jakebailey jakebailey jakebailey approved these changes

@sandersn sandersn sandersn approved these changes

@andrewbranch andrewbranch Awaiting requested review from andrewbranch andrewbranch is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Port custom tslint rules to eslint

3 participants

@JoshuaKGoldberg @jakebailey @sandersn