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: Makefile, check-build.sh clean-ups and perf improvements #28161

Merged
merged 6 commits into from Sep 14, 2023

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Sep 14, 2023

Some users find that make render-docs or make test-docs are too slow for quickly iterating over local changes. This PR contains a few commits to improve things. The first run of either command will still take time, but provided the docs-builder image is present on the system and up-to-date, subsequent runs of make render-docs SKIP_BUILDER_IMAGE=1 or make test-docs SKIP_LINT=1 INCREMENTAL=1 SKIP_BUILDER_IMAGE=1 should go faster. See individual commits for details.

This PR also contains some fixes and clean-ups, in particular for the output of make -C Documentation help.

  • docs: Add SKIP_BUILDER_IMAGE to Makefile to skip rebuilding docs-builder
  • docs: Allow incremental builds for "make html" via env variable
  • docs: Remove dependency on update-helm-values for building HTML
  • docs: Add headings and missing descriptions to "make help" output
  • docs: Reorder targets in Makefile
  • docs: Do not remove _api on "make clean"

Cc @networkop

@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 14, 2023
@qmonnet qmonnet requested a review from a team as a code owner September 14, 2023 13:15
@qmonnet qmonnet changed the title Pr/doc makefile cleanup docs: Makefile, check-build.sh clean-ups and perf improvements Sep 14, 2023
@qmonnet
Copy link
Member Author

qmonnet commented Sep 14, 2023

/test

The Makefile for the documentation makes sure that the docs-builder
image is always present and up-to-date before building the docs. To do
so, it simply rebuilds the image, every time.

This is efficient to make sure the image is up-to-date, but not that
much in terms of build duration. Let's make it possible to skip that
step if the user is confident their image is present and up-to-date.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Allow users to set the INCREMENTAL environment variable to avoid passing
the "-E" option to Sphinx when skipping the linters. This way, we can
have Sphinx do incremental builds instead of re-reading and re-writing
all files. This can be used for quick iterations on a local setup.

There is no need to make "-E" conditional for the spell checker run,
given that the spell checker seems to always re-read all files anyway.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
The documentation build targets, including html, have a dependency on
update-helm-values. There is no clear reason for that: this seems to be
for historical reasons.

- Initially, the dependency was on $(HELM_VALUES), because the file with
  Helm values had not been committed to the Git repo at the time, so we
  needed to ensure that it was generated first.

- When reorganising checks for Helm values in 6167f8a ("docs: check
  updates for the Helm reference"), we replaced it with
  "update-helm-values" which looked cleaner; but we don't need to run
  the _check_ at this stage, only to make sure the _file_ itself is
  available.

Now that the file is part of the repository, there's no need to check
for its existence, and we can simply remove the target dependency (to
align it with other files that receive automatic updates, such as
codeowners.rst or crdlist.rst).

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Let's make sure all Makefile targets are properly documented, and nicely
displayed when running "make help".

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Move around targets inside of Documentation/Makefile to have them
displayed in a more sensible order on "make help".

    $ make help

    Usage:
      make <target>

    Targets (default: "html")

    Development Images
      base-image                    Build the docs-base image for updating the requirements.txt file.
      builder-image                 Build the docs-builder image for rendering and checking the documentation.

    Auto-generated Contents Updates and Validation
      api-flaggen                   Update the table of API flags restrictions.
      update-cmdref                 Update the command reference documents (agent, bugtool, operators, etc.).
      update-codeowners             Update the description of the code owner teams.
      update-crdlist                Update the list of CRDs.
      update-helm-values            Update the Helm reference documentation.
      check                         Validate command and Helm references, policy examples, and others.

    Build
      html                          Check documentation and render it under the specified format.
      epub                          Check documentation and render it under the specified format.
      latex                         Check documentation and render it under the specified format.
      live-preview                  Build and serve the documentation locally.

    Development
      update-requirements           Regenerate the requirements.txt file from requirements-min/requirements.txt.
      clean                         Clean up all artefacts from documentation.
      help                          Display help for the Makefile.

Also add target update-cmdref to .PHONY.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Since commit 944dddf ("docs: Symlink (instead of copying) the API
reference"), we symlink the _api/ directory (from ../api) instead of
copying it. So there's nothing to clean on that side on "make clean",
otherwise we lose the link.

Fixes: 944dddf ("docs: Symlink (instead of copying) the API reference")
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet
Copy link
Member Author

qmonnet commented Sep 14, 2023

/test

Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

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

Thanks for this, @qmonnet. 🌟 I tested changes locally, and make render-docs SKIP_BUILDER_IMAGE=1 does indeed run much more quickly now. ⏩

@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 14, 2023
@qmonnet qmonnet merged commit 5b77a54 into cilium:main Sep 14, 2023
58 checks passed
@qmonnet qmonnet deleted the pr/doc-makefile-cleanup branch September 14, 2023 21:00
@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

5 participants