Skip to content

fix(Auto-fix): ignore is not applied - #654

Merged
webpro merged 2 commits into
webpro-nl:mainfrom
taro-28:fix-autofix-ignore
May 26, 2024
Merged

fix(Auto-fix): ignore is not applied#654
webpro merged 2 commits into
webpro-nl:mainfrom
taro-28:fix-autofix-ignore

Conversation

@taro-28

@taro-28 taro-28 commented May 26, 2024

Copy link
Copy Markdown
Contributor

Closes #653

Fixed and added test cases for the following cases

  • ignore.
  • ignoreDependencies`
  • use ignoreWorkspaces for ignore
  • tags for ignore

Since ignoreMembers is not yet supported by Auto-fix and I thought ignoreBinaries was an option unrelated to Auto-fix, I did not include either of them in the test case.

@taro-28

taro-28 commented May 26, 2024

Copy link
Copy Markdown
Contributor Author

Sorry, I'll check the CI error.

@taro-28
taro-28 marked this pull request as draft May 26, 2024 10:14
@taro-28

taro-28 commented May 26, 2024

Copy link
Copy Markdown
Contributor Author

Sorry, I'll check the CI error.

I checked and could not reproduce it locally.
It is probably a falky test and seems to be unrelated to the changes in this PR.

@taro-28
taro-28 marked this pull request as ready for review May 26, 2024 10:39

@webpro webpro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks a lot for this PR! Just a nitpick and I'm happy to merge 🙌

Comment thread packages/knip/src/index.ts Outdated
});
if (isType) fixer.addUnusedTypeNode(filePath, exportedItem.fixes);
else fixer.addUnusedExportNode(filePath, exportedItem.fixes);
if (issue) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nitpick: since it's a boolean I'd like to name it isAdded or isIssueAdded

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed! I fixed it to isIssueAdded
fffce74

@webpro

webpro commented May 26, 2024

Copy link
Copy Markdown
Member

Thank you @taro-28, most excellent stuff!

@webpro
webpro merged commit 32ddd71 into webpro-nl:main May 26, 2024
@taro-28
taro-28 deleted the fix-autofix-ignore branch May 26, 2024 13:29
@webpro

webpro commented May 26, 2024

Copy link
Copy Markdown
Member

🚀 This pull request is included in v5.17.0-canary.2. See Release 5.17.0-canary.2 for release notes.

Using Knip in a commercial project? Please consider sponsoring me.

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.

ignore is not applied for Auto-fix

2 participants