Create docs directory with starter pack content

[giuliazanchi]

Adding the /docs/ folder and the .readthedocs.yaml file for integration with RTD.

MULTI-1734

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.93%. Comparing base (1c0f995) to head (ebc25f9).
Report is 39 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3855      +/-   ##
==========================================
- Coverage   89.02%   88.93%   -0.10%     
==========================================
  Files         255      255              
  Lines       14577    14827     +250     
==========================================
+ Hits        12977    13186     +209     
- Misses       1600     1641      +41     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[giuliazanchi]

@edibotopic could you review this PR ?

[edibotopic]

LGTM Giulia. I think this should just be merged to get RTD recognising the .readthedocs.yaml and then you should be able to work on preview branches on RTD thereafter (hopefully).

There seems to be some failed lint checks relating to whitespace in a LaTeX template file in .sphinx (outside of your direct control) and the conf.py (under your control).

You might be able to address these issues in the config file if your team wants the /docs/ subdir to be linted also but the .sphinx at least should probably be ignored by the action, if that can be configured.

Any failed builds on local testing of the PR were most likely due to the requirements.txt being in the /docs/.gitignore, which meant that it didn't push from your local machine, where everything had worked previously. You have fixed this now.

I tested this PR locally and everything works fine. There are warnings because of links to non-existent files in the index.rst but that will be resolved once you merge the Multipass docs source files and replace the index file.

[giuliazanchi]

Thanks, Shayne @edibotopic !

[ricab]

Thank you for the PR @giuliazanchi! I can't believe we're finally doing this 😃 I have a few fixes for missing newlines, along with one or two questions, but otherwise it all looks sane to my (unknowing) eyes.

Could you also please describe how to build/use this new folder again? Perhaps we could add that to our read me, next to the other build instructions, WDYT?

As for the failing linting, I think I am missing something. Could you please clarify what prevents fixing those errors?

BTW, thank you also for your review @edibotopic and sorry for stealing your time over to Multipass :) That's my fault, I told Giulia I would feel more comfortable with your stamp on this one.

[ricab]

Suggested change

	}
	}

[edibotopic]

Just for reference, most of the code in .sphinx comes directly from the upstream starter pack.
So any formatting changes like this and others below will likely be undone on subsequent updates, unless the changes are submitted upstream (if they would warrant that). I'm not sure if everyone is using the same linter, mind.

[ricab]

IMO, as long as any code needs to be copied and adopted by a repo, it should adhere to its standards. Of course, the best would be to avoid any copying at all. If nothing else, we could have a submodule pointing to the starter pack along with the necessary links. Or, better, just clone the starter pack at build time.

[edibotopic]

Thanks @ricab -- I see your point. It's up to you and your team to decide what's most important to prioritise and whether this should be a blocker or an improvement to plan for later.

[ricab]

To avoid any misunderstanding, I did not mean to propose that a submodule- or build-based approach be implemented in this PR. I would love to see it, but I agree that can be pursued separately.

Failing that though, PRs should just comply with the norm (here and in future updates). This is a priority for sure. Which is why I don't understand the reluctance to just fix these basic things.

[edibotopic]

It's not reluctance. The main original point I was making was that the changes being proposed needed to be considered in the context of the upstream dependency and any future updates that might override them. I think you have made some sound proposals on that and I don't object to any of these changes in the meantime.

[ricab]

Great, I am glad we've clarified that. Thank you again for all your involvement and help on this!

[ricab]

Suggested change

	}
	}

[ricab]

Suggested change

	</header>
	</header>

[ricab]

Suggested change

	}
	}

[ricab]

Suggested change

	sphinx-last-updated-by-git
	sphinx-last-updated-by-git

[ricab]

Is this intended for the time being?

[edibotopic]

These are defaults. I think the localhost one sometimes helps prevent erroneous errors on local builds.

[ricab]

I was referring to the ACME line specifically. I thought it would be only a default, which is why I asked. In the template, this would presumably serve as an example of what could be added. In this instantiation, I don't see a purpose. As a rule of thumb, pointless LoC are best avoided 😉

[edibotopic]

Yes, indeed, I think the ACME wildcard link is just an example. The only reason to keep it as this point is that this documentation set is still being bootstrapped, so it could be useful as reference; however, I would have no objection to it being deleted.

[edibotopic]

@ricab

Could you also please describe how to build/use this new folder again? Perhaps we could add that to our read me, next to the other build instructions, WDYT?

Local builds are based on a makefile. Running make alone will give you a set of commands. make run will create a virtual environment and install the Python dependencies before serving the docs to localhost (presuming there's no errors). Before submitting a PR for review, it's generally good practice to run make spelling and make linkcheck to avoid CI surprises. Eventually, these docs should also be able to trigger automatic PR previews in the GitHub PR interface, but that takes a little setup (it's worth it).

As for the failing linting, I think I am missing something. Could you please clarify what prevents fixing those errors?

I'm not sure about this. Is the linter a part of the Multipass CI pipeline? I don't think we run a linter on docs files in my projects. For the LaTeX template, for example, this is something built-in to the starter pack. So, you could fix it to stop the linter issues but that would be temporary, and you'd have to apply the same fix again after the next update. So if anything it would be better to fix any issues there in the upstream repo. They seem to be just whitespace issues, so I'm sure they'd be easy to fix. I think it's probably better for people to have a formatter in their editor to catch these things rather than the CI, if possible.

BTW, thank you also for your review @edibotopic and sorry for stealing your time over to Multipass :) That's my fault, I told Giulia I would feel more comfortable with your stamp on this one.

No problem. Happy to help.

[ricab]

Thank you for the build instructions. They failed for me at first, but that was probably because of leftover cruft from the last attempt. It worked after a make clean. I still think it would be good to add simple instructions to our readme.

Before submitting a PR for review, it's generally good practice to run make spelling and make linkcheck to avoid CI surprises.

Good to know. We should probably add something to CI to make sure we don't forget.

Eventually, these docs should also be able to trigger automatic PR previews in the GitHub PR interface, but that takes a little setup (it's worth it).

Yeah, that'd be nice :)

Is the linter a part of the Multipass CI pipeline?

Yes, linting is the first check that our CI runs on PRs. The failing step here is just git diff --check <your-base-commit>. That's not tied to any specific code linter, it is just common practice for working on text files with git.

They seem to be just whitespace issues, so I'm sure they'd be easy to fix.

Exactly: just git rebase --whitespace=fix main. And +1 for fixing upstream too, it would avoid manual steps when adopting code into other repos (although ideally nothing would need to be duplicated/synced at all). A related command might be more useful there: git apply --whitespace=fix <patch>.

I think it's probably better for people to have a formatter in their editor to catch these things rather than the CI, if possible.

I am all for configuring editors to do this automatically and we've recently agreed to do so in the Multipass team. However, I don't think that excludes CI. Things can still get through (as in this PR), because people have different editors, miss the memo, forget configurations in new systems, are new or external contributors, etc. The linting step in CI thus acts as a safety net.

[edibotopic]

@ricab

I think it's probably better for people to have a formatter in their editor to catch these things rather than the CI, if possible.

I am all for configuring editors to do this automatically and we've recently agreed to do so in the Multipass team. However, I don't think that excludes CI. Things can still get through (as in this PR), because people have different editors, miss the memo, forget configurations in new systems, are new or external contributors, etc. The linting step in CI thus acts as a safety net.

Yes, I wasn't saying to replace CI completely. My point was that if (and I don't think this will be the case) the /docs/ dir or /docs/.sphinx dir were to be excluded from linting because it wasn't deemed of immediate value, then editor/CLI formatters could be adopted by the people working on the docs.

I raised the option just in case getting the repo connected to Read the Docs, so that the final configurations and docs migration could be completed in a subsequent PR, was the priority right now.

[ricab]

@edibotopic don't get me wrong, this is a priority. For me, for the Multipass team, certainly for Giulia, who authored the PR. But being a priority doesn't give a PR a pass out of review or basic contribution standards. Especially when adhering is one git command away. Surely you understand that would be a slippery slope and a fast track to degraded quality.

That said, there might be extraordinary difficulties or justification I was unaware of, which is why I asked. So far I have been unable to recognize them. Template updates can still be "fixed" in the same way. If running that extra command when syncing is too much of a burden, the source can be fixed, or the need to sync in the first place.

[edibotopic]

@edibotopic don't get me wrong, this is a priority. For me, for the Multipass team, certainly for Giulia, who authored the PR. But being a priority doesn't give a PR a pass out of review or basic contribution standards. Especially when adhering is one git command away. Surely you understand that would be a slippery slope and a fast track to degraded quality.

That said, there might be extraordinary difficulties or justification I was unaware of, which is why I asked. So far I have been unable to recognize them. Template updates can still be "fixed" in the same way. If running that extra command when syncing is too much of a burden, the source can be fixed, or the need to sync in the first place.

@ricab I didn't know what context you had for the starter pack and just tried to provide some so that you didn't waste any effort. I have no problem with you maintaining the project and it's quality as you see fit and am not challenging that at all. So please proceed based on your judgement. I hadn't considered some of the solutions you proposed, so thanks for suggesting them.

[ricab]

Of course, no worries. I just felt I should clarify my perspective and goal as reviewer here.

I had very little context on the starter pack and your comments helped indeed. It is sort of like a template to boostrap things, right? At least I don't think I will forget how to build the docs anymore 🙂

[giuliazanchi]

Converting back to draft as I'm still debugging some issues on RTD