Code sharing between crawl and runspider command

[jay24rajput]

Implemented a new file common_commands for reusing the add_options and process_option command previously used in crawl.py and runspider.py

Fixes #4548

[Gallaecio]

Instead of creating a new file, what about using commands/__init__.py?

[jay24rajput]

Instead of creating a new file, what about using commands/__init__.py?

There is actually same function i.e add_options and process_options in commands/__init__.py file. And these functions initialize the parser with some common options.

In case we add the code to the __init__.py file, the options will be displayed for each file, which might not be relevant there. Thus, I created a new file so has to avoid the above redundancy.

[codecov]

Codecov Report

Merging #4552 into master will increase coverage by 0.19%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #4552      +/-   ##
==========================================
+ Coverage   84.55%   84.74%   +0.19%     
==========================================
  Files         164      164              
  Lines        9923    10011      +88     
  Branches     1475     1513      +38     
==========================================
+ Hits         8390     8484      +94     
+ Misses       1266     1263       -3     
+ Partials      267      264       -3     

Impacted Files	Coverage Δ
scrapy/commands/__init__.py	62.31% <68.75%> (+1.20%)	⬆️
scrapy/commands/crawl.py	35.00% <100.00%> (+6.42%)	⬆️
scrapy/commands/runspider.py	89.13% <100.00%> (+5.52%)	⬆️
scrapy/robotstxt.py	90.19% <0.00%> (-7.34%)	⬇️
scrapy/cmdline.py	68.00% <0.00%> (+0.25%)	⬆️
scrapy/item.py	100.00% <0.00%> (+1.42%)	⬆️
scrapy/shell.py	72.32% <0.00%> (+4.10%)	⬆️
scrapy/spiders/crawl.py	97.24% <0.00%> (+4.77%)	⬆️

[Gallaecio]

There is actually same function i.e add_options and process_options in commands/__init__.py file. And these functions initialize the parser with some common options.

In case we add the code to the __init__.py file, the options will be displayed for each file, which might not be relevant there. Thus, I created a new file so has to avoid the above redundancy.

I’m not sure what you mean by “the options will be displayed for each file”.

Those two objects are methods of a class, not module-level functions. If you rename your new CommonCommands class to something else (e.g. BaseRunSpiderCommand), you could just move that class to the __init__.py file without any conflict.

[jay24rajput]

Alright! I sort of had a confusion. I will move the entire class to __init__.py and will make both the crawl.py and runspider.py call the newly created class.

[jay24rajput]

@Gallaecio codecov for the patch is failing. Can you please guide me as to how I can fix it.

Also please let me know if anything else needs to be rectified in the previous commit.

[Gallaecio]

Style comments.

[Gallaecio]

Feel free to ignore codecov in this case. You are modifying lines that are not covered by unit tests, so obviously your changes include lines not covered by unit tests, but given how straightforward these changes are, I don’t think we should consider test coverage part of the scope of these changes.

[jay24rajput]

@Gallaecio is there anything else that needs to be done here?

[elacuesta]

Many thanks @jay24rajput! I left a suggestion to simplify a docstring, but it's not a blocker.

[jay24rajput]

@elacuesta @Gallaecio the checks haven't completed yet! Any solutions?

[noviluni]

Hi @jay24rajput it is working, the error is not related to the code but to connection errors. However, if you want to relaunch the check you can close and open again this PR.