Skip to content

Update tutorial.rst: clarify how to run spider #6128

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

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

JessA-L
Copy link
Contributor

@JessA-L JessA-L commented Nov 2, 2023

When I used the tutorial, I was confused about how to run the spider again. I later realized that I never exited the Scrapy shell.

This edit should provide more guidance to future users of Scrapy and the tutorial.

Add clarifying language
@wRAR
Copy link
Member

wRAR commented Nov 2, 2023

I'm not convinced we should expect users to not understand the difference between console commands and Python code :-/

@JessA-L
Copy link
Contributor Author

JessA-L commented Nov 2, 2023

Hi, thanks for your quick reply.

I’m proposing this change because I had to troubleshoot it myself. Another fellow student had issues at this section as well. Further, Scrapy is used by others who aren’t super familiar with the command line, like those in the marketing field. The tutorial begins with references to basic python instruction, so it seems like it targets unexperienced users.

@wRAR
Copy link
Member

wRAR commented Nov 2, 2023

Yeah, it makes sense, but in that case I'd want something more general or something written in the beginning of the tutorial that explains these basics, as the proposed "exit the scrapy shell" line would be OK in a differently structured doc (one that expected a single terminal opened in a specific directory and showed exact commands one after another, with "now open the scrapy shell", "now close it and run scrapy crawl" etc. steps) but looks out of place to me in this somewhat more abstract tutorial.

@Gallaecio what do you think?

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.

I think the proposed changes are great. While I agree with @wRAR’s points for general documentation, I think in a tutorial specifically hand-holding is more important than not repeating yourself.

Copy link

codecov bot commented Nov 3, 2023

Codecov Report

Merging #6128 (9cdb150) into master (593bfd8) will increase coverage by 0.07%.
Report is 3 commits behind head on master.
The diff coverage is n/a.

❗ Current head 9cdb150 differs from pull request most recent head a3cd068. Consider uploading reports for the commit a3cd068 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6128      +/-   ##
==========================================
+ Coverage   88.71%   88.78%   +0.07%     
==========================================
  Files         160      160              
  Lines       11428    11428              
  Branches     1862     1862              
==========================================
+ Hits        10138    10146       +8     
+ Misses        979      973       -6     
+ Partials      311      309       -2     

see 1 file with indirect coverage changes

@wRAR
Copy link
Member

wRAR commented Nov 3, 2023

Thanks!

@Gallaecio Gallaecio merged commit 6587556 into scrapy:master Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants