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

guide: use more root-relative links, and replace duplication with templated markdown #310

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

daprilik
Copy link
Contributor

This PR contains a few related changes:

  1. Convert most intra-doc links to use root-relative paths, instead of file-relative paths. i.e: instead of ../../foo, just use /path/to/foo, which is likely to be a more stable in the face of content being reorganized.
  2. Instead of having a bunch of _images/ folders, just have a single _images/ folder, which can be referred to via /_images/image.png regardless where the md file is in the repo.
  3. Avoid instances of duplication between certain pages by introducing a new _fragments/ folder, and leveraging mdbook's built-in {{ #include path/to/content.md }} directives.

These are all relatively straightforward changes, though I did run in rust-lang/mdBook#1512 while working on 3.

I quickly hacked together a workaround using the existing mdbook-openvmm-shim preprocessor we have, which did the trick.
Of course, it'd be nice if this was fixed upstream, but we'll cross that bridge another day...

@daprilik daprilik requested a review from a team as a code owner November 13, 2024 01:20
@mattkur
Copy link
Contributor

mattkur commented Nov 13, 2024

Can we enforce any of these conventions with CI (or did this PR do that and I just missed it)?

You can install it locally by running: `cargo install cargo-nextest --locked`

See the [cargo-nextest](https://nexte.st/) documentation for more info.
{{ #include /_fragments/nextest_tip.md }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you have to put the fragment inside the admonish block here, at the consuming side? (Or did you do it this way so that consumers of the fragment can decide if it's in an admonishment or not).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately, this is a limitation of how the current templating infrastructure works :(

I'm sure with a bit more custom preprocessing / hacking I could maybe get this working, but at a certain point, you gotta throw in the towel and bend to the tool you're using

@daprilik
Copy link
Contributor Author

Can we enforce any of these conventions with CI (or did this PR do that and I just missed it)?

We don't have much guide-level linting infrastructure, but we could certainly look into extending our xtask fmt house-rules infrastructure to enforce certain conventions in the Guide/. I would probably file a separate tracking issue for those sorts of improvements, so we can start writing down the sorts of Guide lints we'd want to see.

@daprilik daprilik merged commit b09016f into microsoft:main Nov 13, 2024
24 checks passed
daprilik added a commit to daprilik/openvmm that referenced this pull request Nov 13, 2024
daprilik added a commit that referenced this pull request Nov 13, 2024
…tion with templated markdown (#310)" (#322)

Turns out absolute links don't work correctly:
rust-lang/mdBook#1764

This means that the guide is busted when published on openvmm.dev/guide,
even if it works correctly locally. Unfortunate...
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