-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
[docs-infra] Fix Vale no longer working #46029
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
Conversation
Netlify deploy previewhttps://deploy-preview-46029--material-ui.netlify.app/ Bundle size report |
8f4440d
to
88641be
Compare
554ea23
to
b439d0e
Compare
Sparkline Chart: Sparkline chart | ||
Gauge Chart: Gauge chart | ||
Treemap Chart: Treemap chart |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those one are correct. we agree on the fact that those item does not need an extra "chart"
Sparkline Chart: Sparkline chart | |
Gauge Chart: Gauge chart | |
Treemap Chart: Treemap chart | |
Sparkline Chart: Sparkline | |
Gauge Chart: Gauge | |
Treemap Chart: Treemap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the change because of https://github.com/mui/mui-x/pull/17602/files#r2067364155. It doesn't seem that we can do anything about it. We have to allow it.
@@ -1,5 +1,5 @@ | |||
--- | |||
title: 'The 2019 Material UI Developer Survey: here's what we discovered' | |||
title: "The 2019 Material UI Developer Survey: here's what we discovered" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is their a specific reason why using "
instead of '
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other valid YAML syntax would be:
title: "The 2019 Material UI Developer Survey: here's what we discovered" | |
title: 'The 2019 Material UI Developer Survey: here''s what we discovered' |
I was lazy, didn't add support for the ''
escaping.
components: Alert, AlertTitle | ||
githubLabel: 'component: alert' | ||
packageName: '@mui/lab' | ||
waiAria: https://www.w3.org/TR/wai-aria-practices/#alert | ||
authors: | ||
['foo', 'bar'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing those lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal is to keep the test more focused on what we try to test, this is unrelated.
I've open a PR to test it on MUI X mui/mui-x#17602 |
I was working on a quick fix, mui/mui-x#17538. There I noticed in MUI X that Vale was reporting 100 errors https://github.com/mui/mui-x/actions/runs/14686411363/job/41215568904.
but it was strange as I don't get those in Material UI nor if I run the script locally. However, in Material UI, it just crashes: https://github.com/mui/material-ui/actions/runs/14682247158/job/41206345211
as it turned out, it was a pile of problems, and broken for some time:
🧹 done. The last thing would be to have 1. the Vale VS Code extension use the binary available locally 2. an official npm version, maybe one day.
Next:
.github/workflows/vale-action.yml
, updatepackage.json
, updatepnpm-workspace.yaml
.