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: Run rstcheck on the README.rst #26454

Merged
merged 2 commits into from Jun 26, 2023
Merged

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Jun 23, 2023

Commits:

  • README.rst: Fix RST duplicate explicit target names
  • docs: Run rstcheck on README.rst, too.

This is to make sure we catch issues on the formatting of the releases table before merging, see #13366.

When trying to run rstcheck on the README.rst, it reports two sets of
duplicate explicit target names. Indeed, we have two links that each
appear twice, with the same URLs and target names. This is not a good
practice, given that it makes more locations to update if we ever need
to update one of these URLs. Instead, let's keep the URLs once and have
the references point to it. This way we can also move the URLs out of
the RST paragraph, to make the text file easier to read.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Let's run rstcheck on the README.rst file in addition to the
documentation sources. One motivation is to help catch issues on the
releases table before merging. We update this table with a script after
cutting new releases, and it has occasionally led to formatting issues
in the past.

Fixes: cilium#13366
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/ci This PR makes changes to the CI. labels Jun 23, 2023
@qmonnet qmonnet requested a review from a team as a code owner June 23, 2023 09:39
@qmonnet qmonnet requested a review from learnitall June 23, 2023 09:39
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.

My apologies for the delay, I looked at this last week but forgot to approve! LGTM!

@borkmann borkmann added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 26, 2023
@borkmann borkmann merged commit 5951d1d into cilium:main Jun 26, 2023
56 checks passed
@qmonnet qmonnet deleted the pr/rstcheck-readme branch June 27, 2023 08:44
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. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants