Skip to content

docs: change DOWNLOAD_DELAY description format #5761

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 2 commits into from
Dec 15, 2022
Merged

Conversation

yolile
Copy link
Contributor

@yolile yolile commented Dec 15, 2022

To be consistent with the other settings' descriptions

To be consistent with the other settings' descriptions
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 do not appreciate an improvement in the rewording.

The use of “Note” as a paragraph seems like a change for the worse. Sphinx has a directive, note, for that. But I am not sure a note format is right here, as this is the main place in the documentation where this spider attribute is covered.

The use of :attr: could be an improvement if it was pointing somewhere, but I don’t think it does.

@yolile
Copy link
Contributor Author

yolile commented Dec 15, 2022

Ops, sorry, fixed now. My proposal is just to be consistent with the other DOWNLOAD_ settings description format, e.g. https://docs.scrapy.org/en/latest/topics/settings.html#download-timeout

@Gallaecio
Copy link
Member

Makes sense. And I see that even though :attr: does not link anything, the formatting itself is a welcome change.

Thank you!

@Gallaecio
Copy link
Member

Note before merging: someone needs to make sure it renders OK, usually the note text starts on the same line as the directive, and I am not sure the syntax used here will work.

@wRAR wRAR merged commit 034dc8f into scrapy:master Dec 15, 2022
@yolile yolile deleted the patch-1 branch December 15, 2022 20:04
@wRAR
Copy link
Member

wRAR commented Dec 15, 2022

@Gallaecio web-poet uses the same syntax (even with the empty line in at least one place).

@wRAR
Copy link
Member

wRAR commented Dec 15, 2022

Actually even the same docs/topics/settings.rst includes the same syntax.

Unless I've misunderstood your concerns.

@yolile
Copy link
Contributor Author

yolile commented Dec 15, 2022

It renders fine for me (in case this helps):
image

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