-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[docs] Fix broken links #43144
[docs] Fix broken links #43144
Conversation
I'm curious why the check reports links on |
Because it does not really look at material-ui/docs/scripts/reportBrokenLinks.js Lines 213 to 217 in bacd2db
The true testing is about relative links material-ui/docs/scripts/reportBrokenLinks.js Line 205 in bacd2db
|
It looks like a good candidate to use the |
f7eef73
to
33d82be
Compare
These changes look OK, but IMO it would be better to fix the link-check script first (to ensure it catches them) |
The fix is here #43195 The script was completely broken due to what I assume is a breaking change in the @michaldudak Do you know if it's feasible to add test on scripts? None of the docs scripts are tested. I don't know what to do to add one such that this does not happened again |
The cleanest way I can think of would be to extract the script logic into packages-internal/scripts, test it there and leave just the CLI in the scripts directory. |
Hey @alexfauquette, sorry for the delay on this PR. I updated it, and it's ready for review again. |
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.
As long as the section anchor exist, I'm happy with it ;)
**Second**, you can add [custom variants](/material-ui/customization/theme-components/#creating-new-component-variants) to the theme, overriding the CSS for specific component prop combinations. | ||
**Second**, you can add [custom variants](https://v5.mui.com/material-ui/customization/theme-components/#creating-new-component-variants) to the theme, overriding the CSS for specific component prop combinations. |
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.
Interesting tradeoff. A blog post like v5 isn't read much in the future, the page view distribution overtime is very much an exp(2-2x), so we shouldn't have to maintain those links 👍
Fix broken links reported in https://app.circleci.com/pipelines/github/mui/base-ui/2800/workflows/addeece5-c28e-48b7-9f29-fb49d9bf1327/jobs/17721.
All of these are from core to core, so the link check broke. It would be good to fix this check in this same PR.