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

unused_ignore_directives doesn't work on stdlib/3rd party imports #518

Closed
DetachHead opened this issue Jan 7, 2025 · 3 comments · Fixed by #563
Closed

unused_ignore_directives doesn't work on stdlib/3rd party imports #518

DetachHead opened this issue Jan 7, 2025 · 3 comments · Fixed by #563

Comments

@DetachHead
Copy link

# tach.toml
[rules]
unused_ignore_directives = "error"
# tach-ignore
from typing import Iterable
> tach check
✅ All modules validated!
@DetachHead
Copy link
Author

i guess fixing this would need to account for tach-ignore comments that are intended for tach check-external rather than tach check

@DetachHead
Copy link
Author

maybe tach check-external can instead be part of tach check which can be enabled or disabled using a rule, (like with unused_ignore_directives):

# tach.toml
[rules]
check_external = true

i imagine doing this would also address gauge-sh/tach-vscode#17

@emdoyle
Copy link
Member

emdoyle commented Jan 23, 2025

For now, it is simpler to hide tach-ignore comments which refer to imports that are irrelevant. This won't flag the comment in your example during tach check, but it will flag it during tach check-external.

This should be easy to reason about for now, but we want to hold off on using global config to enable the external use case like this. For the VSCE issue, I think an LSP setting to enable the external check makes sense though. After some more work to give consistent structure to external dependency errors, I can work on that.

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 a pull request may close this issue.

2 participants