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 109 forks source link

[Docs] Add vale ini and styles for docs linting #385

Closed stichbury closed 3 months ago

stichbury commented 3 months ago

Description

This PR adds the .vale files to the /docs folders of vizro-core and vizro-ai so that I can manually run vale on docs pages as I work on them.

At some point I need to work with @l0uden to add this to our CI. We can do this as part of the PR but equally, as long as these files are in place, I can just invoke Vale for now.

Screenshot

Notice

stichbury commented 3 months ago

I'm not sure what the problem with the vizro-ai docs build is but I haven't changed anything in the docs build, so will investigate separately. It may be that we can/should put a single set of Vale files in the root of vizro to avoid duplication across core and ai folders?

maxschulz-COL commented 3 months ago

Yes, ideally we would only have one folder with those files!

How are you using it, how is it invoved? Can we add it to pre-commit such that it gets invoked with our hatch run lint command. Then it should be not a problem in CI either, as this is done automatically!

stichbury commented 3 months ago

How are you using it, how is it invoved? Can we add it to pre-commit such that it gets invoked with our hatch run lint command. Then it should be not a problem in CI either, as this is done automatically!

At present, I'm just invoking vale from the same folder as vale.ini and pointing at an individual file or folder to target. It's probably worth thinking how we want to use it across our docs -- we could either take the big hit of fixing everything in one, and then run across the full docs, or we could apply it just to any markdown file in a PR, which I think is what Kedro do https://github.com/kedro-org/kedro. I don't really mind which as long as it's available to check anything I'm working on. I think it can become annoying (it's still not fully tweaked for our purposes, so will be moaning about things) so I'm happy to spend more time on the styles to get them adjusted to Vizro lexicon.

As I'm not an engineer and not very well-versed in setting up this kind of tooling, please can I pass this one over to whoever could finish the PR from that perspective?

If you want to run across the full docs set, I'll need to put some time in place to read the issues and tweak styles or docs to get things passing.

maxschulz-COL commented 3 months ago

How are you using it, how is it invoved? Can we add it to pre-commit such that it gets invoked with our hatch run lint command. Then it should be not a problem in CI either, as this is done automatically!

At present, I'm just invoking vale from the same folder as vale.ini and pointing at an individual file or folder to target. It's probably worth thinking how we want to use it across our docs -- we could either take the big hit of fixing everything in one, and then run across the full docs, or we could apply it just to any markdown file in a PR, which I think is what Kedro do https://github.com/kedro-org/kedro. I don't really mind which as long as it's available to check anything I'm working on. I think it can become annoying (it's still not fully tweaked for our purposes, so will be moaning about things) so I'm happy to spend more time on the styles to get them adjusted to Vizro lexicon.

As I'm not an engineer and not very well-versed in setting up this kind of tooling, please can I pass this one over to whoever could finish the PR from that perspective?

If you want to run across the full docs set, I'll need to put some time in place to read the issues and tweak styles or docs to get things passing.

Ok I see :)

Can you estimate how long the full pass would take? I think if we make this PR the introduction of vale, then there is no harm in it sitting here a bit longer. Ultimately I would like it to be another linter, because this is the only way to achieve consistency. Let me take a quick look what we can do, can I work into this PR?

stichbury commented 3 months ago

@maxschulz-COL Yes please feel free to commit on this branch and take it on. I think a full pass would be less than a day of tweaking.

maxschulz-COL commented 3 months ago

@maxschulz-COL Yes please feel free to commit on this branch and take it on. I think a full pass would be less than a day of tweaking.

Ok then let's go for that option, I will setup the pre-commit version of it

maxschulz-COL commented 3 months ago

@stichbury I think you are good to go - vale can be invoked via hatch run lint, and it will also do so in CI :) Let me know if there are still any issues.

So the plan is to fix the existing vale errors, warnings and suggestions, tweak the setting a little, and then have our docs perfectly styled all the way 😎 ?

maxschulz-COL commented 3 months ago

@stichbury Please double check if I messed up anything with the styles folder - I just copied over the one from the vizro-core, it's now all in a central place

stichbury commented 3 months ago

Thank you, that is all fine! It all looks great, and I'll make a branch from this one to fix up the docs errors and tweak styles to get all building cleanly as we go forward. Thanks for making the changes to get it in the tooling!