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 deprecated Spider.make_requests_from_url method #4178

Merged
merged 1 commit into from Aug 20, 2021

Conversation

elacuesta
Copy link
Member

@elacuesta elacuesta commented Nov 20, 2019

As mentioned in #4170 (comment)

This method was deprecated in #1728, released in 1.4.0

Should we also remove scrapy.utils.deprecate.method_is_overridden? It was introduced in the above PR and it's not currently used anywhere else, but I think its purpose is still valid.

Closes #4356

@codecov
Copy link

codecov bot commented Nov 20, 2019

Codecov Report

Merging #4178 (d63cc68) into master (5a75b14) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head d63cc68 differs from pull request most recent head 91f8144. Consider uploading reports for the commit 91f8144 to get more accurate results

@@            Coverage Diff             @@
##           master    #4178      +/-   ##
==========================================
- Coverage   88.09%   88.07%   -0.02%     
==========================================
  Files         162      162              
  Lines       10345    10335      -10     
  Branches     1504     1502       -2     
==========================================
- Hits         9113     9103      -10     
  Misses        964      964              
  Partials      268      268              
Impacted Files Coverage Δ
scrapy/spiders/__init__.py 100.00% <100.00%> (ø)

@Gallaecio
Copy link
Member

Should we also remove scrapy.utils.deprecate.method_is_overridden?

Maybe deprecate it first, just in case?

@elacuesta
Copy link
Member Author

Makes sense, but just to be clear, it's not that I want to remove it. I actually think it's still valid and we might need it at some point.
I figured that since the function is not used anymore after this change, a discussion about its removal would start and I thought it would be faster if I just raised the question to begin with - Perhaps it was too much early optimization 😅

Copy link
Member

@Gallaecio Gallaecio left a comment

Choose a reason for hiding this comment

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

OK, I guess we can leave that for a separate ticket.

@nyov
Copy link
Contributor

nyov commented Mar 7, 2020

And it hasn't been properly deprecated in Scrapy 2.0, either? 😭

I hope it might be ready for removal at the big anniversary (1.4.0 > 2.4.0), so we can close this in 2024ish? ;)
I hope this will work: #4412

@wRAR
Copy link
Member

wRAR commented Sep 2, 2020

What's the status of this?

@kmike
Copy link
Member

kmike commented Sep 30, 2020

I think we should aim to remove this method 1 year after the release which contains #4412, as per https://docs.scrapy.org/en/master/versioning.html#deprecation-policy.

It looks like this release was Scrapy 2.1 which happened 2020-04-24, so we should be waiting until April 2021 before merging this (assuming there is going to be a release in May).

@elacuesta elacuesta added this to the 2.6 milestone Mar 29, 2021
@elacuesta elacuesta force-pushed the remove_spider_make_requests_from_url branch from ec19daa to 91f8144 Compare April 7, 2021 18:17
@wRAR wRAR merged commit 731f2d3 into scrapy:master Aug 20, 2021
@elacuesta elacuesta deleted the remove_spider_make_requests_from_url branch August 20, 2021 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should we delete the deprecated code in Scrapy 2.0?
5 participants