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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Update Sphinx and its dependencies, Cilium theme #28172

Merged
merged 4 commits into from Sep 18, 2023

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Sep 15, 2023

I rebased our fork of the Sphinx theme here: https://github.com/cilium/sphinx_rtd_theme/tree/cilium/rebase-2023-09

Now we can use this newer version in the docs. We can also update a bunch of other dependencies that were stuck because of the docutils version formerly required by the theme.

  • docs: Drop is_html5_writer_available() in cilium_external_links.py
  • docs: Update Sphinx (7.1.2) and dependencies
  • docs: Re-enable Sphinx output in check-build.sh
  • ci: update docs-builder

v2 of #28171, from Cilium repo for the deployments to work.

Function is_html5_writer_available() is deprecated, its invocation is
unnecessary (the writer _is_ available in the versions of Sphinx we're
using), and it has been removed in more recent versions of Sphinx. Let's
get rid of it.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Now that we have rebased our theme onto the upstream version, we can use
newer versions of docutils, which allows us to update Sphinx and a bunch
of dependencies. We're still limited to docutils 0.18 because of
sphinx-tabs, though.

One immediate improvement is that we get working links for the value
types in the gRPC reference.

There are newer versions of Sphinx, websupport, or some second-level
dependencies available (currently 7.2.6 and 1.2.6, respectively), but
Netlify doesn't know about them yet and fails to build the previews.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
In the past, we disabled Sphinx's output in check-build.sh. This was
because of the way we handle warnings: in order to have them printed at
the end of the logs, we print them to a file, we filter them, and print
the outstanding ones at the end of the output. Keeping the standard
output meant that warnings or errors wouldn't appear in the middle of
this output, and warnings that we filtered out wouldn't appear at all,
which could be confusing.

With the recent Sphinx update, we are able to filter the warnings from
MyST more efficiently, directly from conf.py, and to get rid of the
filters.

Let's take advantage of this to re-enable the output from Sphinx. While
it's not necessary to build the docs, it's occasionally helpful to debug
the setup when things don't go as expected.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet qmonnet added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.12 needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Sep 15, 2023
@qmonnet qmonnet temporarily deployed to docs-builder September 15, 2023 00:17 — with GitHub Actions Inactive
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet qmonnet temporarily deployed to docs-builder September 15, 2023 00:21 — with GitHub Actions Inactive
@qmonnet
Copy link
Member Author

qmonnet commented Sep 15, 2023

/test

@qmonnet
Copy link
Member Author

qmonnet commented Sep 15, 2023

Netlify preview

@qmonnet qmonnet marked this pull request as ready for review September 15, 2023 00:27
@qmonnet qmonnet requested review from a team as code owners September 15, 2023 00:27
Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

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

FWIW, we don't need to run /test here, the documentation workflow runs automatically on pull_request events.

@qmonnet
Copy link
Member Author

qmonnet commented Sep 15, 2023

@nbusseneau My understanding is that I need to /test anyway if I want the checks to be all green and the PR to either get the green badge or get mergeable by me, directly. Non-relevant tests for docs PR are skipped anyway.

Thanks a lot for the review!

Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

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

Great! I took a look at the netifly preview and nothing that I could see stood out or looked weird.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Sep 15, 2023
@nbusseneau
Copy link
Member

My understanding is that I need to /test anyway if I want the checks to be all green and the PR to either get the green badge or get mergeable by me, directly. Non-relevant tests for docs PR are skipped anyway.

True, I just typically set it to ready-to-merge and state why /test is not necessary in a comment, but both approaches work!

@julianwiedmann julianwiedmann merged commit 2f7fbfe into main Sep 18, 2023
206 checks passed
@julianwiedmann julianwiedmann deleted the pr/qmonnet/sphinx-deps-update branch September 18, 2023 08:51
@doniacld doniacld mentioned this pull request Sep 22, 2023
10 tasks
@doniacld doniacld added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Sep 22, 2023
@giorio94 giorio94 mentioned this pull request Sep 26, 2023
22 tasks
@giorio94 giorio94 added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Sep 26, 2023
@giorio94 giorio94 mentioned this pull request Sep 26, 2023
12 tasks
@aanm aanm added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. backport-pending/1.12 labels Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants