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.

Sign up for GitHub

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

Jump to bottom

Add configuration for Read the Docs build and pin docs requirements #888

Merged
merged 3 commits into from Oct 28, 2021
Merged

Add configuration for Read the Docs build and pin docs requirements #888

merged 3 commits into from Oct 28, 2021

Conversation

samdoran
Copy link
Contributor

@samdoran samdoran commented Oct 26, 2021 •
edited

Due to a recent release of docutils 0.18, our doc build is broken.
Add configuration for Read the Docs and pip docs requirements to ensure builds will not spontaneously break in the future.
Update tox configuration to match the build done by Read the Docs.

@samdoran samdoran mentioned this pull request Oct 27, 2021
Merged
@samdoran samdoran marked this pull request as ready for review October 27, 2021 20:46
@samdoran samdoran requested a review from a team as a code owner October 27, 2021 20:46
@samdoran samdoran changed the title Add configuration for Read the Docs builds and pin docs requirements Add configuration for Read the Docs build and pin docs requirements Oct 27, 2021
webknjaz
webknjaz reviewed Oct 27, 2021
View reviewed changes
docs/requirements.in Outdated Show resolved Hide resolved
@samdoran
Add read the docs configuration
fe07a6e 
This lets us specify the version of deps we use since currently it is using
a very old version of Sphinx for builds.
@samdoran
Pin docs requirements
c581051
@samdoran samdoran force-pushed the docs/pin-requirements branch 2 times, most recently from e40dacb to 196eff7 Compare October 27, 2021 21:49
@samdoran samdoran added the gate label Oct 28, 2021
ansible-zuul[bot]
ansible-zuul bot approved these changes Oct 28, 2021
View reviewed changes
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

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

LGTM!

webknjaz
webknjaz reviewed Oct 28, 2021
View reviewed changes
tox.ini Outdated Show resolved Hide resolved
@ansible-zuul ansible-zuul bot removed the gate label Oct 28, 2021
webknjaz
webknjaz reviewed Oct 28, 2021
View reviewed changes
tox.ini Outdated Show resolved Hide resolved
@samdoran
Copy link
Contributor Author

@webknjaz Thanks for all your feedback. It's super helpful!

@samdoran samdoran added the gate label Oct 28, 2021
@webknjaz
Copy link
Member

@samdoran just to clarify — did you explicitly decide not to use -n or did you miss that suggestion?

@samdoran
Copy link
Contributor Author

@webknjaz I did miss that, but it's not in the RTD build command and --keep-going seemed to actually do the right thing without -n in my testing.

@samdoran
Copy link
Contributor Author

Ok, I think I see the distinction. The warning I was seeing was due to a missing dependency in the virtual environment, not a missing doc reference:

WARNING: autodoc: failed to import module 'exceptions' from module 'ansible_runner'; the following exception was raised:
No module named 'pexpect'

@ansible-zuul ansible-zuul bot removed the gate label Oct 28, 2021
@samdoran samdoran added the gate label Oct 28, 2021
@webknjaz
Copy link
Member

webknjaz commented Oct 28, 2021 •
edited

--keep-going seemed to actually do the right thing without -n in my testing.

To clarify: adding -W makes more warnings show up, they all are printed out in the output. -n on top of that makes Sphinx fail on the first warning it sees so you would only see one warning, fix it and have to rerun to see the next one. When you add --keep-going to the mix, it will not fail on the first warning but it will print out everything produced by -W, finish the build and only then return an error.

webknjaz
webknjaz reviewed Oct 28, 2021
View reviewed changes
tox.ini Outdated
commands =
sphinx-build -E -W -d docs/build/doctrees -b html docs docs/build/html
sphinx-build -T -E -W -n --keep-going {tty:--color} -j auto -d docs/build/doctrees -b html docs docs/build/html
Copy link
Member

Choose a reason for hiding this comment

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

Side question: why is the caching disabled with -E? Doesn't it conflict with the speed-up that -d would've caused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about that one.

@samdoran
Update tox configuration to match RTD docs build
c5b3637 
Keep the build type as html, though, for easier local navigation.
@ansible-zuul ansible-zuul bot removed the gate label Oct 28, 2021
@samdoran samdoran added the gate label Oct 28, 2021
@webknjaz
Copy link
Member

Was -n causing problems locally?

@ansible-zuul ansible-zuul bot merged commit cd69e4d into ansible:devel Oct 28, 2021
@samdoran samdoran deleted the docs/pin-requirements branch October 28, 2021 22:35
@samdoran
Copy link
Contributor Author

Yes. And the CI job failed too. Evidently we have some docs issues to resolve.

@webknjaz
Copy link
Member

@samdoran you can add all reference errors to nitpick_ignore in conf.py. Then, you'd be able to keep -n and fix those over time.

samdoran pushed a commit to samdoran/ansible-runner that referenced this pull request Oct 29, 2021
@ansible-zuul @samdoran
Add configuration for Read the Docs build and pin docs requirements (a…
7ad9421 
…nsible#888)

Due to a recent release of docutils 0.18, our doc build is broken.
Add configuration for Read the Docs and pip docs requirements to ensure builds will not spontaneously break in the future.
Update tox configuration to match the build done by Read the Docs.

Reviewed-by: David Shrewsbury <None>
Reviewed-by: Sviatoslav Sydorenko <webknjaz+github/profile@redhat.com>
Reviewed-by: Sam Doran <sdoran@redhat.com>
Reviewed-by: Shane McDonald <me@shanemcd.com>

(cherry picked from commit cd69e4d)
@samdoran samdoran mentioned this pull request Oct 29, 2021
Merged
samdoran pushed a commit to samdoran/ansible-runner that referenced this pull request Oct 29, 2021
@ansible-zuul @samdoran
Add configuration for Read the Docs build and pin docs requirements (a…
4f93f06 
…nsible#888)

Due to a recent release of docutils 0.18, our doc build is broken.
Add configuration for Read the Docs and pip docs requirements to ensure builds will not spontaneously break in the future.
Update tox configuration to match the build done by Read the Docs.

Reviewed-by: David Shrewsbury <None>
Reviewed-by: Sviatoslav Sydorenko <webknjaz+github/profile@redhat.com>
Reviewed-by: Sam Doran <sdoran@redhat.com>
Reviewed-by: Shane McDonald <me@shanemcd.com>

(cherry picked from commit cd69e4d)
@samdoran samdoran mentioned this pull request Oct 29, 2021
Merged
ansible-zuul bot added a commit that referenced this pull request Oct 29, 2021
@ansible-zuul
Merge pull request #892 from samdoran/backport/888/release_2.1
1f786d0 
[release_2.1] Add configuration for Read the Docs build and pin docs requirements (#888)

Backport of #888 for Ansible Runner 2.1.

Reviewed-by: David Shrewsbury <None>
Reviewed-by: None <None>
ansible-zuul bot added a commit that referenced this pull request Oct 29, 2021
@ansible-zuul
Merge pull request #893 from samdoran/backport/888/release_2.0
bba1641 
[release_2.0] Add configuration for Read the Docs build and pin docs requirements (#888)

Backport of #888 for Ansible Runner 2.0.

Reviewed-by: David Shrewsbury <None>
Reviewed-by: None <None>
@webknjaz
Copy link
Member

Linking this for history: readthedocs/readthedocs.org#8616

@Shrews Shrews mentioned this pull request Feb 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webknjaz webknjaz webknjaz approved these changes

@shanemcd shanemcd shanemcd approved these changes

@Shrews Shrews Shrews approved these changes

@ansible-zuul ansible-zuul[bot] ansible-zuul approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@samdoran @webknjaz @shanemcd @Shrews