Import paths #2733

[marlenachatzigrigoriou]

@Gallaecio please check this file and let me know if this is the way that I should carry this issue, so I will be able to continue with the other files.
Fixes #2733.

[codecov]

Codecov Report

Merging #5099 (6acad38) into master (016c7e9) will decrease coverage by 4.10%.
The diff coverage is n/a.

❗ Current head 6acad38 differs from pull request most recent head c91b3f6. Consider uploading reports for the commit c91b3f6 to get more accurate results

@@            Coverage Diff             @@
##           master    #5099      +/-   ##
==========================================
- Coverage   88.19%   84.09%   -4.11%     
==========================================
  Files         162      162              
  Lines       10497    10497              
  Branches     1517     1517              
==========================================
- Hits         9258     8827     -431     
- Misses        965     1409     +444     
+ Partials      274      261      -13     

Impacted Files	Coverage Δ
scrapy/core/http2/stream.py	27.01% <0.00%> (-64.37%)	⬇️
scrapy/pipelines/images.py	28.07% <0.00%> (-62.29%)	⬇️
scrapy/core/http2/agent.py	36.14% <0.00%> (-60.25%)	⬇️
scrapy/core/downloader/handlers/http2.py	43.42% <0.00%> (-56.58%)	⬇️
scrapy/core/http2/protocol.py	34.17% <0.00%> (-49.25%)	⬇️
scrapy/utils/ssl.py	53.65% <0.00%> (-17.08%)	⬇️
scrapy/utils/asyncgen.py	83.33% <0.00%> (-16.67%)	⬇️
scrapy/core/downloader/contextfactory.py	75.92% <0.00%> (-11.12%)	⬇️
scrapy/utils/test.py	50.00% <0.00%> (-10.94%)	⬇️
scrapy/utils/reactor.py	75.00% <0.00%> (-6.67%)	⬇️
... and 6 more

[Gallaecio]

That’s the idea. However, mind that this pull request includes unrelated commits that you probably dragged from another pull request.

[Gallaecio]

Could you find out why the pull request diff shows so many changes? I suspect you might have accidentally changed the line ending of the files you edited.

[marlenachatzigrigoriou]

You are right @Gallaecio. I had accidentally changed the autocrlf configuration. Should I delete this commit and create a new one?

[Gallaecio]

Either that or simply create a new one. In pull requests, at least in Scrapy, we are not strict at all with commit cleanliness, and in fact not rebasing (and hence force-pushing) can make it easier for reviewers.

Mind that you also have conflicts with master now, you you will need to merge the latest master branch into your branch and resolve any conflict.

[Gallaecio]

We need to be careful when editing news.rst. Unlike other pages in the documentation, this page covers past versions where the code (e.g. imports available) might have been different. For example, it seems there was no scrapy.Item in 0.8, so the last change in news.rst is definitely wrong.

Since one of the goals of this change is to keep all documentation links to different import paths working, I would suggest not changing the news.rst page at all.

[Gallaecio]

The changes look good to me. I think we can merge as soon as we address #5099 (comment) and the issue reported by CI (sounds like .. class:: scrapy.Item may be used more than once now, if so we need to remove one of those usages).

[marlenachatzigrigoriou]

Hello @Gallaecio, could you guide me on how to fix the lines-ending error? Because, I have tried the git checkout <commit_hash> <file_name> command on commit e821e2d on spider.rst file, and then I committed again the changes on 36704e9 and a59d7dc where the lines-ending looks right, but spriders.rst in "files changed" section of this pr has still the false line ending. I suppose that the same is going to happen if I apply the command on practices.rst.
I could also revert the whole commit e821e2d, but it contains many files..

[Gallaecio]

@marlenachatzigrigoriou I must say I’m not sure myself how to do it. I suggest you try https://stackoverflow.com/a/21014534/939364. If that does not work either, let me know, I’ll give it a try myself.

[marlenachatzigrigoriou]

@Gallaecio I think the way you suggested is not for this case. " for the commits you haven’t pushed yet" and I've already committed and pushed the changes in commits that contain more than one file.
git checkout <commit_hash> <file_name> is the most proper way, I found, to do it, but the lines-ending is still wrong.

[Gallaecio]

I’ll give it a try when I get a chance.

[mmitropoulou]

Hello @Gallaecio! @marlenachatzigrigoriou and I have fixed the lines ending problem in spiders.rst & practices.rst. It would be nice if you could check the files, so the pull request may hopefully get merged. Thank you!

[Gallaecio]

Looks great to me, thank you for such a thorough change.