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

doc: decode the objects.inv file #12701

Merged
merged 1 commit into from
Jan 18, 2024
Merged

Conversation

ru-fu
Copy link
Contributor

@ru-fu ru-fu commented Jan 5, 2024

Sphinx creates an objects.inv file that contains a mapping of IDs and locations in the docs. The UI needs this mapping to be able to resolve links within the descriptions of config options.

The UI would require additional dependencies to decode the objects.inv file, so providing the decoded version with the documentation is easier.

@ru-fu ru-fu requested a review from edlerd January 5, 2024 16:45
@ru-fu ru-fu requested a review from tomponline as a code owner January 5, 2024 16:45
@ru-fu
Copy link
Contributor Author

ru-fu commented Jan 5, 2024

@gabrielmougard The doc build for the snap uses the general doc target, doesn't it?
So this change will apply to the snap build as well?

Copy link
Contributor

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

LGTM, just one clarification below.


.PHONY: doc-incremental
doc-incremental:
@echo "Build the documentation"
. $(SPHINXENV) ; LOCAL_SPHINX_BUILD=True sphinx-build -c doc/ -b dirhtml doc/ doc/html/ -d doc/.sphinx/.doctrees -w doc/.sphinx/warnings.txt

.PHONY: doc-objects
doc-objects:
. $(SPHINXENV); cd doc/html; python3 -m sphinx.ext.intersphinx 'objects.inv' > objects.inv.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume the resulting objects.inv.txt will be shipped along the docs -- just like the objects.inv file. Is that correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the plan - but I just noticed that this will only work for the local docs, not the online version (because RTD doesn't use the Makefile). Let me add it for RTD as well ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@github-actions github-actions bot added the Documentation Documentation needs updating label Jan 5, 2024
@ru-fu ru-fu force-pushed the decode-objects-inv branch 2 times, most recently from 05cddfa to eeaa08c Compare January 5, 2024 17:17
@gabrielmougard
Copy link
Contributor

@gabrielmougard The doc build for the snap uses the general doc target, doesn't it? So this change will apply to the snap build as well?

yes. Here is the snap doc build step that ships the built doc:

if [ "$(uname -m)" != "riscv64" ]; then
        # Build the static website
        make doc

        # Copy the static website
        mkdir -p "${CRAFT_STAGE}/share/lxd-documentation"
        cp -a doc/html/. "${CRAFT_STAGE}/share/lxd-documentation/"
fi

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

@ru-fu how big is this new decoded file ?

@ru-fu
Copy link
Contributor Author

ru-fu commented Jan 8, 2024

@ru-fu how big is this new decoded file ?

Currently 170 KiB.


.PHONY: doc-incremental
doc-incremental:
@echo "Build the documentation"
. $(SPHINXENV) ; LOCAL_SPHINX_BUILD=True sphinx-build -c doc/ -b dirhtml doc/ doc/html/ -d doc/.sphinx/.doctrees -w doc/.sphinx/warnings.txt

.PHONY: doc-objects
doc-objects:
. $(SPHINXENV); cd doc/html; python3 -m sphinx.ext.intersphinx 'objects.inv' > objects.inv.txt
Copy link
Member

Choose a reason for hiding this comment

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

What format is this new file in @ru-fu ?

Also, is it possible to add a comment here explaining what its intended to be used for? I.e to be consumed by the UI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a text file.
And do you mean a comment in the Makefile? I have an explanation in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah in the makefile thanks.

Are the UI team able to parse the text file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@edlerd suggested this change, so I would assume so. ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Are the UI team able to parse the text file?

We have to write a parser for it, but that should be trivial. So: Yes.

Copy link
Member

Choose a reason for hiding this comment

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

@edlerd you've seen the text format and happy with it?

Copy link
Member

Choose a reason for hiding this comment

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

Excellent thanks @edlerd

Sphinx creates an objects.inv file that contains a mapping of IDs and
locations in the docs. The UI needs this mapping to be able to resolve
links within the descriptions of config options.

The UI would require additional dependencies to decode the objects.inv
file, so providing the decoded version with the documentation is
easier.

Signed-off-by: Ruth Fuchss <ruth.fuchss@canonical.com>
@tomponline
Copy link
Member

LGTM!

@tomponline tomponline merged commit dbeb673 into canonical:main Jan 18, 2024
25 of 26 checks passed
@ru-fu ru-fu deleted the decode-objects-inv branch January 18, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants