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

Fix Mermaid diagram rendering #3875

Merged
merged 15 commits into from
Apr 4, 2024
Merged

Fix Mermaid diagram rendering #3875

merged 15 commits into from
Apr 4, 2024

Conversation

liamaharon
Copy link
Contributor

@liamaharon liamaharon commented Mar 28, 2024

Closes #2977

The issue appears to stem from the aquamarine crate failing to render diagrams in re-exported crates.

e.g. as raised here, diagrams would render at frame_support::traits::Hooks but not the re-exported doc frame::traits::Hooks, even if I added aquamarine as a frame crate dependency.

To resolve this, I followed advice in mersinvald/aquamarine#20 to instead render mermaid diagrams directly using JS by adding an after-content.js.


Also fixes compile warnings, enables --all-features and disallows future warnings in CI.

@liamaharon liamaharon added the T11-documentation This PR/Issue is related to documentation. label Mar 28, 2024
@liamaharon liamaharon requested review from a team as code owners March 28, 2024 09:09
@paritytech-review-bot paritytech-review-bot bot requested a review from a team March 28, 2024 09:15
@liamaharon liamaharon added the R0-silent Changes should not be mentioned in any release notes label Mar 28, 2024
@bkchr
Copy link
Member

bkchr commented Mar 31, 2024

But this will still fail on docs.rs?

We should think on how to fix this there as well. Setting RUSTDOCFLAGS is also possible: https://github.com/rust-lang/docs.rs/blob/8b19841b449244d79976c29ae24534af3d7e8c82/templates/core/Cargo.toml.example#L42

@liamaharon
Copy link
Contributor Author

But this will still fail on docs.rs?

We should think on how to fix this there as well. Setting RUSTDOCFLAGS is also possible: https://github.com/rust-lang/docs.rs/blob/8b19841b449244d79976c29ae24534af3d7e8c82/templates/core/Cargo.toml.example#L42

Would we need to set RUSTDOCFLAGS and include the after script in every crate? Maybe it's easier to open an issue downstream in the aquamarine crate and try get it fixed there..

@liamaharon
Copy link
Contributor Author

So this pr doesn't break crates.io and we can merge it I'm going to restore the aquamarine crate and use it alongside the new after-content script.

@kianenigma
Copy link
Contributor

docs.rs

fwiw we have generally broader issues with docs.rs, especially with publishing polkadot-sdk-docs crate, see: sam0x17/docify#22

It might then therefore be a better tradeoff to say for now we only focus on self-hosted rust docs.

@liamaharon liamaharon added this pull request to the merge queue Apr 4, 2024
Merged via the queue into master with commit 0ef37c7 Apr 4, 2024
129 of 134 checks passed
@liamaharon liamaharon deleted the liam-fix-aquamarine branch April 4, 2024 11:56
@liamaharon
Copy link
Contributor Author

Confirmed fixed in prod 🥳 https://paritytech.github.io/polkadot-sdk/master/polkadot_sdk_frame/traits/trait.Hooks.html

Ank4n pushed a commit that referenced this pull request Apr 9, 2024
Closes #2977

The issue appears to stem from the `aquamarine` crate failing to render
diagrams in re-exported crates.

e.g. as raised
[here](#2977), diagrams
would render at `frame_support::traits::Hooks` but not the re-exported
doc `frame::traits::Hooks`, even if I added `aquamarine` as a `frame`
crate dependency.

To resolve this, I followed advice in
mersinvald/aquamarine#20 to instead render
mermaid diagrams directly using JS by adding an `after-content.js`.

---

Also fixes compile warnings, enables `--all-features` and disallows
future warnings in CI.

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Apr 9, 2024
Closes paritytech#2977

The issue appears to stem from the `aquamarine` crate failing to render
diagrams in re-exported crates.

e.g. as raised
[here](paritytech#2977), diagrams
would render at `frame_support::traits::Hooks` but not the re-exported
doc `frame::traits::Hooks`, even if I added `aquamarine` as a `frame`
crate dependency.

To resolve this, I followed advice in
mersinvald/aquamarine#20 to instead render
mermaid diagrams directly using JS by adding an `after-content.js`.

---

Also fixes compile warnings, enables `--all-features` and disallows
future warnings in CI.

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T11-documentation This PR/Issue is related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doc: mermaid diagram not rendering
5 participants