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

Should scrapy.utils.python.WeakKeyCache be deprecated? #4684

Closed
noviluni opened this issue Jul 16, 2020 · 5 comments · Fixed by #4701
Closed

Should scrapy.utils.python.WeakKeyCache be deprecated? #4684

noviluni opened this issue Jul 16, 2020 · 5 comments · Fixed by #4701
Labels

Comments

@noviluni
Copy link
Member

noviluni commented Jul 16, 2020

Summary

I can't find any reference to scrapy.utils.python.WeakKeyCache (except in tests) and I think that it should be deprecated.

Motivation

Keeping non-used and non-documented code makes the codebase harder to maintain.

Describe alternatives you've considered

As it's tested we could keep it, but I don't think it's a good idea.

@wRAR
Copy link
Contributor

wRAR commented Jul 16, 2020

Looks like its (last) usage was removed in 332bf3b

@kshitijcode
Copy link
Contributor

kshitijcode commented Jul 28, 2020

I can clean this up. Is it up for grabs? Can someone assign this to me, please?
@Gallaecio @wRAR

This would be my first open-source contribution :)

@Gallaecio
Copy link
Member

Gallaecio commented Jul 28, 2020

@kshitijcode No need to ask first, feel free to go ahead! 🙂

@kshitijcode
Copy link
Contributor

kshitijcode commented Jul 29, 2020

@Gallaecio @wRAR
I have raised the PR with the necessary changes: #4701
But I am not sure what is failing the tests.

Tests pass successfully locally after adding -W ignore::pytest.PytestDeprecationWarning. to tox.ini
I am not really sure whats breaking in Azure Pipelines.

@wRAR
Copy link
Contributor

wRAR commented Jul 29, 2020

@kshitijcode the test failure is unrelated to your changes, I've created #4702 to track it

kshitijcode pushed a commit to kshitijcode/scrapy that referenced this issue Jul 29, 2020
kshitijcode pushed a commit to kshitijcode/scrapy that referenced this issue Jul 30, 2020
@wRAR wRAR closed this as completed in #4701 Aug 5, 2020
wRAR added a commit that referenced this issue Aug 5, 2020
Code cleanup scrapy.utils.python.WeakKeyCache #4684
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants