mckinsey / vizro

Vizro is a toolkit for creating modular data visualization applications.
https://vizro.readthedocs.io/en/stable/
Apache License 2.0
2.47k stars 111 forks source link

Create a documentation style guide to apply across docs #325

Closed stichbury closed 4 months ago

stichbury commented 4 months ago

Description

This is part of the work to apply a light edit and consistency check to docs. I've created a page with a lightweight guide for us to agree upon before I start editing.

You can read it here!

Related issues

I've created a separate issue to install Vale (which is a text linter) to check text changes in CI and catch when the guidelines are not observed. This isn't urgent but a nice-to-have.

Notice

stichbury commented 4 months ago

Any suggestions for Vizro-specific terminology guidelines I should include? So far I've just captured that controls should be capitalized when used as code names (e.g. Containers) but otherwise lower case. Is that correct?

Anything else?

antonymilne commented 4 months ago

Love this! ❤️

One thing I would love us to have a rule on because it's extremely inconsistent is when and how often we should link to the API reference. Take any page: https://vizro.readthedocs.io/en/stable/pages/user_guides/container/

You see our models Container, Page etc. sometimes link directly to the API docs, sometimes they don't. Initially we've said that we would link it only once, so the first time we mention it throughout that page. But we do it very inconsistently 😅

Would love to get your opinion on this! Personally I don't mind that much, if we reference only once per page, once per section or we have to reference it everywhere. Just some consistency would be nice 😄

Another thing that's a bit of an issue at the moment is where a link saying Container should point to - the API reference or the user guide on using containers?

Both links are useful in different contexts but I'm not sure we always link to the right one, and there's not currently a link that takes you from the API docs on Container to the relevant part of the user guide I think. So if you click on a link to Container and you end up on the API docs then you're sort of stuck there.

Possibly Container should always go to the API docs, and "container" (just text) should go to the user guide? But maybe that's too subtle...

My hunch here is that most of the time most people are more interested in the narrative docs than the API ones, but maybe the tracking data will say otherwise. I generally regard the narrative docs as more important than the API ones (which are really just a nice to have), so in general I'd be inclined to prioritise cross-linking between guide pages over the API ones. But happy to go with whatever seems best to you @stichbury.

antonymilne commented 4 months ago

@stichbury

I've created a separate issue to install Vale (which is a text linter) to check text changes in CI and catch when the guidelines are not observed. This isn't urgent but a nice-to-have.

This sounds great. I see that vale now officially has pre-commit hooks available on their repo (https://github.com/errata-ai/vale/pull/558) so it should be very easy to add to our CI setup, where everything is configured through pre-commit. All our linting/formatting/checking/whatever tools are done using pre-commit and it works very smoothly in general.

maxschulz-COL commented 4 months ago

Another thing that's a bit of an issue at the moment is where a link saying Container should point to - the API reference or the user guide on using containers?

Maybe we can make it such that every time we say Container model, we link to the API docs, but otherwise we link to the guides.

Additionally we could start having a link in the docstring to the relevant user guide, like that one isn't stuck!

And before I forget, one thing to be included, and also attributed is our docs framework https://diataxis.fr/. I sent you a link on our internal docs guide that we had all along 😂 we just all forgot about it haha

stichbury commented 4 months ago

Thanks for the feedback! I've addressed that all (except the diataxis structuring, which I need to consider some more as I go through the docs, since I think that requires a bit of a different level of thought to these language-style rules). I've committed the new version and will set to auto-merge since there's a CI issue (unrelated) at present I think.

huong-li-nguyen commented 4 months ago

Thank you! Yes, @l0uden is aware of it. He's currently trying to fix it! 🚀