Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Deprecated now-duplicate rules from TSLint 5.12.0 (bumped devDependency to 5.12.0) #686

Merged
merged 10 commits into from
Dec 27, 2018
Merged

Deprecated now-duplicate rules from TSLint 5.12.0 (bumped devDependency to 5.12.0) #686

merged 10 commits into from
Dec 27, 2018

Conversation

JoshuaKGoldberg
Copy link

PR checklist

IllusionMH
IllusionMH suggested changes Dec 19, 2018
View reviewed changes
Copy link
Contributor

@IllusionMH IllusionMH left a comment

Choose a reason for hiding this comment

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

Deprecation message for no-increment-decrement should be fixed.

Other notes are about metadata for tslint-warnings.csv (which honestly do not understand fully) that has unexpected differences from same now-deprecated rules.
At this point it is hard for me to tell if changes are required for moved and newly added rules.

src/noIncrementDecrementRule.ts Outdated Show resolved Hide resolved
additional_rule_metadata.json Outdated Show resolved Hide resolved
additional_rule_metadata.json Outdated Show resolved Hide resolved
additional_rule_metadata.json Outdated Show resolved Hide resolved
IllusionMH
IllusionMH suggested changes Dec 20, 2018
View reviewed changes
Copy link
Contributor

@IllusionMH IllusionMH left a comment

Choose a reason for hiding this comment

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

Only problem is not updated tslint-warnings.csv. Everything else looks good 👍

tslint-warnings.csv Outdated Show resolved Hide resolved
This was referenced Dec 21, 2018
Merged
Merged
Merged
JoshuaKGoldberg
JoshuaKGoldberg commented Dec 26, 2018
View reviewed changes
Copy link
Author

@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.

@IllusionMH I added "recommendation": "false," to rules that would be a breaking change in the recommended ruleset. Otherwise I think it's good to go?

@JoshuaKGoldberg JoshuaKGoldberg added the PR: Waiting for Reviewer A repository maintainer should take a look at the pull request soon! label Dec 26, 2018
IllusionMH
IllusionMH suggested changes Dec 26, 2018
View reviewed changes
Copy link
Contributor

@IllusionMH IllusionMH 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 recommended_ruleset.json was not updated and incorrect severity.

Otherwise good 👍

additional_rule_metadata.json Outdated Show resolved Hide resolved
recommended_ruleset.js Outdated Show resolved Hide resolved
@IllusionMH IllusionMH added PR: Waiting for Author Changes have been requested that the pull request author should address. and removed PR: Waiting for Reviewer A repository maintainer should take a look at the pull request soon! labels Dec 26, 2018
@IllusionMH
Copy link
Contributor

And question indirectly related to this PR:
no-unnecessary-bind has special checks for underscore methods. Is there a need to re-implement rule with underscore specific checks or they are redundant?

@JoshuaKGoldberg
Copy link
Author

JoshuaKGoldberg commented Dec 26, 2018 •
edited
Loading

underscore specific checks

IMO they're ok to remove. If someone would like to add those in, that should be added as a configurable parameter to core's unnecessary-bind.

@JoshuaKGoldberg JoshuaKGoldberg added PR: Waiting for Reviewer A repository maintainer should take a look at the pull request soon! and removed PR: Waiting for Author Changes have been requested that the pull request author should address. labels Dec 26, 2018
IllusionMH
IllusionMH reviewed Dec 26, 2018
View reviewed changes
recommended_ruleset.js Outdated Show resolved Hide resolved
@IllusionMH IllusionMH added PR: Waiting for Author Changes have been requested that the pull request author should address. and removed PR: Waiting for Reviewer A repository maintainer should take a look at the pull request soon! labels Dec 27, 2018
@JoshuaKGoldberg JoshuaKGoldberg mentioned this pull request Dec 27, 2018
Closed
@JoshuaKGoldberg JoshuaKGoldberg added PR: Waiting for Reviewer A repository maintainer should take a look at the pull request soon! and removed PR: Waiting for Author Changes have been requested that the pull request author should address. PR: Waiting for Reviewer A repository maintainer should take a look at the pull request soon! labels Dec 27, 2018
@JoshuaKGoldberg JoshuaKGoldberg merged commit 168d874 into microsoft:master Dec 27, 2018
@JoshuaKGoldberg JoshuaKGoldberg deleted the tslint-5.12.0 branch December 27, 2018 15:08
@JoshuaKGoldberg JoshuaKGoldberg mentioned this pull request Dec 27, 2018
Closed
@IllusionMH IllusionMH mentioned this pull request Dec 27, 2018
Closed
@mastermatt mastermatt mentioned this pull request Feb 19, 2019
Closed
@IllusionMH IllusionMH added this to the 6.1.0-beta milestone Feb 19, 2019
apawast pushed a commit to lupine86/tslint-microsoft-contrib that referenced this pull request Feb 26, 2019
@apawast
Deprecated now-duplicate rules from TSLint 5.12.0 (bumped devDependen…
1cb905b 
…cy to 5.12.0) (microsoft#686)

* Deprecated now-duplicate rules from TSLint 5.12.0 (bumped devDependency to 5.12.0)

Fixes microsoft#684. Fixes microsoft#685.

* Added missing deprecation notices in README.md

* Update noUnnecessaryBindRule.ts

* Corrected changes in package-lock.json and metadata

* Fixed recommended ruleset and tslint warnings

* Removed breaking rule changes

* Fixed recommendation typos

* Updated metadata files

* Reverted react-a11y-accessible-headings line
@falkenhawk falkenhawk mentioned this pull request Sep 5, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@IllusionMH IllusionMH IllusionMH approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
6.1.0-beta
Development

Successfully merging this pull request may close these issues.

Update to TSLint 5.12.0 PR builds on Node 6 fail during linting
2 participants
@JoshuaKGoldberg @IllusionMH