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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs: Update example Sphinx .readthedocs.yaml #10481

Merged
merged 2 commits into from Jun 28, 2023

Conversation

benjaoming
Copy link
Contributor

@benjaoming benjaoming commented Jun 27, 2023

@humitos related to #10478 - yet another place to manage tool versions, albeit not so critical.


馃摎 Documentation previews 馃摎

@benjaoming benjaoming requested a review from humitos June 27, 2023 12:14
@benjaoming benjaoming requested a review from a team as a code owner June 27, 2023 12:14
@benjaoming benjaoming removed the request for review from ericholscher June 27, 2023 12:15
# You use the dirhtml builder for cleaner URLs
# builder: "dirhtml"
# Fail on all warnings to avoid broken references
# fail_on_warning: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reasons to add this:

  1. I just had a support inquiry where it seems they have somehow been confused about how to apply this option that was previously available through the UI. I think adding it in the example might help.

  2. By also mentioning fail_on_warning, we have mentioned all options available :) It is a really good feature to be aware of, which I think most Sphinx projects will benefit from.

Copy link
Member

Choose a reason for hiding this comment

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

I think these are good hints and makes sense to mention these config keys here 馃憤馃徏 . I'm not sold on the copy for the comments but I'm not how to suggest something, tho. I'm pinging @ericholscher here who is my English mentor 馃槢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, that sentence was pretty ugly, fixed it now.

We could also not explain things and use direct links to the docs. E.g. https://docs.readthedocs.io/en/stable/config-file/v2.html#sphinx-builder

Copy link
Member

Choose a reason for hiding this comment

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

This copy looks good to me 馃憤

# You use the dirhtml builder for cleaner URLs
# builder: "dirhtml"
# Fail on all warnings to avoid broken references
# fail_on_warning: true
Copy link
Member

Choose a reason for hiding this comment

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

I think these are good hints and makes sense to mention these config keys here 馃憤馃徏 . I'm not sold on the copy for the comments but I'm not how to suggest something, tho. I'm pinging @ericholscher here who is my English mentor 馃槢

@benjaoming benjaoming merged commit 3aff517 into main Jun 28, 2023
7 checks passed
@benjaoming benjaoming deleted the docs/example-readthedocsyaml-update branch June 28, 2023 11:44
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.

None yet

3 participants