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

Remove find_dependent_paths and depenent path cleanup #85

Merged
merged 1 commit into from
Oct 19, 2021

Conversation

zero323
Copy link
Contributor

@zero323 zero323 commented Oct 19, 2021

This PR drops dependent files removal logic from runtest and deletes find_dependent_paths method.

Rationale:

The logic for finding dependent paths is clearly broken at the moment (#83) and clearly does more harm than good.

In general, mypy should be able to identify out-of-date cache items and recompute these with any user intervention, so this functionality should be unnecessary for correctness and serve as a mere optimization to keep cache size under control.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

LGTM! Mypy worked differently back in the days 🙂

@sobolevn sobolevn merged commit 1ad40a9 into typeddjango:master Oct 19, 2021
@zero323 zero323 deleted the disable-cache-cleanup branch October 20, 2021 08:51
@zero323
Copy link
Contributor Author

zero323 commented Oct 20, 2021

LGTM!

Thanks @sobolevn

Mypy worked differently back in the days 🙂

I couldn't say ‒ until now I didn't break anything related to cache :)

The idea of cache cleaning / pruning might be still useful, but ti definitely has to be more subtle and with good test coverage that can show use when something breaks. It is also a subject for discussion if, in the current state of things, cleaning test cache is safe

if not self.disable_cache:
for file in self.files:
path = Path(file.path)
self.remove_cache_files(path.with_suffix(""))

It doesn't seem to break anything in an obvious way at the moment, but I'll keep poking around and see what comes up ‒ I hope you don't mind reviewing my silly PRs from time to time ;)

@sobolevn
Copy link
Member

I hope you don't mind reviewing my silly PRs from time to time ;)

I am really happy to review your awesome PRs 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants