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

Migrate Multipass docs to RTD #3916

Draft
wants to merge 112 commits into
base: main
Choose a base branch
from

Conversation

giuliazanchi
Copy link
Contributor

@giuliazanchi giuliazanchi commented Feb 3, 2025

This PR adds the /docs/ folder to the repo, including the documentation files in .md format, all the necessary files to build and render the docs in Sphinx, and the starter pack extension.

To test it out, go into the /docs/ folder and run make clean and then make run, or just make to see the available targets.

@giuliazanchi
Copy link
Contributor Author

Additional info:

The test site on RTD, based on a branch of my forked repo, is here: https://canonical-multipass-docs-migration-test.readthedocs-hosted.com/en/migrate-docs-to-rtd/

The branch of my fork used for this test is: https://github.com/giuliazanchi/multipass/tree/migrate-docs-to-rtd

giuliazanchi and others added 28 commits February 5, 2025 12:51
we have bidirectional streaming rpcs, but we usually send only one
response back on that stream for most operations. therefore, the gui
would only read data from the stream until it got the first reply. but
this introduced errors in which the stream was disposed too early. now
we read everything from the stream, so that issue shouldn't happen.
@giuliazanchi
Copy link
Contributor Author

Hi @levkropp , I did a make clean on my local copy, but the build files are already in the ignore list... so there are no changes to commit. I'll update the instructions above to say that when you clone the repo you first have to make clean and then make run.

@levkropp
Copy link
Contributor

levkropp commented Feb 5, 2025

Hi @levkropp , I did a make clean on my local copy, but the build files are already in the ignore list... so there are no changes to commit. I'll update the instructions above to say that when you clone the repo you first have to make clean and then make run.

Hey @giuliazanchi ! It's looking like make run is working on a fresh clone of the repo, so perhaps it was an issue with my python configuration when I first cloned earlier, or maybe it was related to the state of the repo before the latest commits. Either way I am currently able to run make run immediately without no need for make clean 😀

@giuliazanchi giuliazanchi marked this pull request as ready for review February 6, 2025 11:10
@jibel jibel requested a review from levkropp February 6, 2025 11:10
Copy link

@edibotopic edibotopic left a comment

Choose a reason for hiding this comment

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

LGTM @giuliazanchi

Everything seems to work fine and I didn't notice anything broken.

There are a few things I spotted that you might want to consider here or in a future PR, including:

  • Amending conf.py to add the feedback button
  • Synchronising tabs on pages with multiple tab blocks
  • Removing serial dashes from some page titles and the side nav

I didn't go deep into the content of individual pages but there were no notable problems and all the tests seem to be passing.

docs/conf.py Show resolved Hide resolved

Home <self>
tutorial/index
how-to-guides/index
Copy link

@edibotopic edibotopic Feb 6, 2025

Choose a reason for hiding this comment

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

Suggested change
how-to-guides/index
how-to-guides/index
How-to guides <how-to-guides/index>

I don't think you want titles with multiple dashes like How-to-guides and Command-Line-Interface in the section titles? They weren't in the original MP docs and there is a few of them here.

A change like the one above in docs/index.md enables you to assign the name that will appear in the side navigation (in this case: How-to guides).

Alternatively, you can change the top-level (h1) heading in the how-to-guides/index page, for example, and this should be reflected in the side navigation (as well as the page, of course).

This shouldn't affect the URL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! This is how they came out of the extraction script, I hadn't noticed the dashes in the side navigation. Thanks, I'll fix the H1 titles everywhere.

Select the tab corresponding to your operating system (e.g. Linux) to display the relevant content in each section. Your decision will stick until you select another OS from the drop-down menu.
```
(install-multipass-prerequisites)=
## Check prerequisites
Copy link

@edibotopic edibotopic Feb 6, 2025

Choose a reason for hiding this comment

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

In this section, the tab choices are not synchronised within the page. So, someone has to choose, say, macOS each time as they progress downwards.

If you want the tab choice to affect others on this page (and others) with multiple tab blocks:

  • replace each {tab-set} with {tabs} and each {tab-item} with {group-tab}.

Comment on lines +697 to +701
```{toctree}
:hidden:
:titlesonly:
:maxdepth: 2
:glob:

Choose a reason for hiding this comment

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

Suggested change
```{toctree}
:hidden:
:titlesonly:
:maxdepth: 2
:glob:

I don't think this is necessary?

A side effect of including this is that in the side panel the Tutorial appears like this: Tutorial v (i.e., with a dropdown arrow). The dropdown suggests that there are multiple tutorials but there is only one.

I removed this toctree block: nothing seemed to break, the dropdown disappeared and the tutorial could be clicked as normal.

@giuliazanchi giuliazanchi marked this pull request as draft February 6, 2025 17:24
@giuliazanchi
Copy link
Contributor Author

This PR is now replaced by #3921 to resolve some issues with the commit history

@edibotopic
Copy link

This PR is now replaced by #3921 to resolve some issues with the commit history

I moved my comments there.

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.

9 participants