Skip to content

doc: add checks on docs integration - #1480

Open
elijahgreenstein wants to merge 1 commit into
canonical:mainfrom
elijahgreenstein:integration-tests
Open

doc: add checks on docs integration#1480
elijahgreenstein wants to merge 1 commit into
canonical:mainfrom
elijahgreenstein:integration-tests

Conversation

@elijahgreenstein

Copy link
Copy Markdown
Contributor

The docs integration often depends on overwriting files in the upstream LXD, MicroCeph, and MicroOVN repositories. If those files move or if configuration settings change, it is possible for the build to complete successfully, but with incorrect settings or other issues.

This commit separates the integrate process in the docs Makefile into several steps:

  • integrate-pull clones the upstream repositories and pins them to specific commits
  • integrate-precheck (depends on integrate-pull) confirms that various files are located where expected, and are configured as expected
  • integrate (depends on integrate-precheck) copies and overwrites files, and builds the documentation

The docs integration often depends on overwriting files in the upstream
LXD, MicroCeph, and MicroOVN repositories. If those files move or if
configuration settings change, it is possible for the build to complete
successfully, but with incorrect settings or other issues.

This commit separates the `integrate` process in the docs Makefile into
several steps:

- `integrate-pull` clones the upstream repositories and pins them to
  specific commits
- `integrate-precheck` (depends on `integrate-pull`) confirms that
  various files are located where expected, and are configured as
  expected
- `integrate` (depends on `integrate-precheck`) copies and overwrites
  files, and builds the documentation

Signed-off-by: Elijah Greenstein <elijah.greenstein@canonical.com>
@elijahgreenstein
elijahgreenstein marked this pull request as draft August 6, 2026 20:40
@elijahgreenstein
elijahgreenstein marked this pull request as ready for review August 6, 2026 20:40
@elijahgreenstein

Copy link
Copy Markdown
Contributor Author

Related to #1475, with regard to the noindex checks.

@elijahgreenstein

Copy link
Copy Markdown
Contributor Author

@roosterfish -- these are some checks intended to catch changes to the upstream repositories before building and serving the integrated documentation. As you can see, I've currently set these up so that the build fails if there are any issues. However, I also considered setting the checks up as make target that could be triggered as a GitHub workflow, such that the build could complete regardless of the status of the checks. Do you have any thoughts on these two approaches? Ultimately I decided on having the build fail. If, for example, the MicroOVN header template is moved to a new directory, it's possible for the build to complete but the built docs to be set up incorrectly. I believe this was the kind of issue that we had with MicroCeph in the MicroCloud 2 docs that was resolved with this PR: #1454.

@roosterfish

Copy link
Copy Markdown
Contributor

Do you have any thoughts on these two approaches?

I think it's ok to have a failing build in this case as it indicates we have to update our doc set accordingly.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the docs integration workflow by splitting the existing integrate target into discrete steps so upstream-repo assumptions are validated before files are overwritten and docs are built.

Changes:

  • Split upstream repo fetching into a new integrate-pull target.
  • Add an integrate-precheck target that validates expected upstream file locations and key template/config assumptions before integration.
  • Make integrate depend on integrate-precheck to enforce the validation gate.
Suppressed comments (1)

doc/Makefile:89

  • The fetch/clone logic will run git clone whenever git -C … fetch fails (e.g., transient network error or a local repo issue), which can produce a misleading failure like “destination path already exists” and hide the real fetch problem. Consider cloning only when the repo directory doesn’t exist, and otherwise failing on fetch errors.
integrate-pull:
	# Pull the other repositories
	cd integration/ && ( git -C lxd fetch || git clone https://github.com/canonical/lxd )
	git -C integration/lxd reset --hard $(LXDVERSION)


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread doc/Makefile
Comment on lines 88 to 94
@@ -92,10 +93,72 @@ integrate:
cd integration/ && ( git -C microovn fetch || git clone https://github.com/canonical/microovn )
git -C integration/microovn reset --hard $(MICROOVNVERSION)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems unrelated to me as the PR doesn't change the pinning behavior.

@roosterfish roosterfish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

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.

3 participants