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

[docs-infra] Enforce Vale #226

Open
oliviertassinari opened this issue Sep 2, 2023 · 1 comment
Open

[docs-infra] Enforce Vale #226

oliviertassinari opened this issue Sep 2, 2023 · 1 comment
Labels
enhancement This is not a bug, nor a new feature scope: docs-infra Specific to the docs-infra product test

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 2, 2023

Summary 💡

We now have mui/material-ui#32486 but at this stage, nothing enforces that the codebase is getting better over time.

Motivation 🔦

We don't want to have to do PRs like mui/toolpad#4347.

Solution space

We are using GitHub Action to report issues on the changes but:

  1. Because of an API limit in GitHub, the Vale GitHub Action might fail Action fails even if fail_on_error is false errata-ai/vale-action#89, and because when it fails it's hell to figure out where the errors are, we added continue-on-error: true to ignore it.
  2. Even if we fix this API limit (1.), we still need to update docs-infra that includes new rules, so we need to lint the whole codebase, not only each PR diff. So we truly need to add a vale sync && git ls-files | grep -h -E ".(mdx|md)$" | xargs vale --filter='.Level=="error"' in the CI. Now, the problem is that we import the config with Packages = Google, https://github.com/mui/material-ui/raw/HEAD/docs/mui-vale.zip. We need to get the .zip directly from @mui/monorepo, so it's idempotent (we don't want to break other repositories' main branch when the mono repo updates the Vale rules), but then, the problem is that the vale GitHub Action would fail, we would need to install the npm dependencies.

So overall, I would propose this diff:

diff --git a/.github/workflows/vale-action.yml b/.github/workflows/vale-action.yml
index 555ecbce6..b6e0fe403 100644
--- a/.github/workflows/vale-action.yml
+++ b/.github/workflows/vale-action.yml
@@ -13,6 +13,13 @@ jobs:
       pull-requests: write
     steps:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+      - name: Set up pnpm
+        uses: pnpm/action-setup@fe02b34f77f8bc703788d5817da081398fad5dd2 # v4.0.0
+      - name: Use Node.js 20.x
+        uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0
+        with:
+          node-version: 20
+          cache: 'pnpm' # https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#caching-packages-dependencies
       - uses: errata-ai/vale-action@d89dee975228ae261d22c15adcd03578634d429c # v2.1.1
         continue-on-error: true # GitHub Action flag needed until https://github.com/errata-ai/vale-action/issues/89 is fixed
         with:
diff --git a/.vale.ini b/.vale.ini
index 0d8bbaf0e..e69c4f39e 100644
--- a/.vale.ini
+++ b/.vale.ini
@@ -7,7 +7,7 @@ MinAlertLevel = warning
 # 2. Update/create YAML files
 # 3. Run `pnpm docs:zipRules` to generate the zip files
 # 4. You can test locally by replacing the url with the file path of the generated zip
-Packages = Google, https://github.com/mui/material-ui/raw/HEAD/docs/mui-vale.zip
+Packages = Google, node_modules/@mui/monorepo/docs/mui-vale.zip

 [*.md]
 # Ignore code injections that start with {{.

And then, we can copy https://github.com/mui/material-ui/blob/573028441c7f21a53bdf691be71a14ff0efc261f/.circleci/config.yml#L243

Search keywords:

@oliviertassinari oliviertassinari added new feature New feature or request scope: docs-infra Specific to the docs-infra product labels Sep 2, 2023
@danilo-leal
Copy link

I think @samuelsycamore had created the same issue a while back! :) 👉 mui/material-ui#32486

@oliviertassinari oliviertassinari added duplicate This issue or pull request already exists and removed new feature New feature or request scope: docs-infra Specific to the docs-infra product labels Sep 4, 2023
@oliviertassinari oliviertassinari marked this as a duplicate of mui/material-ui#32486 Sep 4, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2023
@oliviertassinari oliviertassinari changed the title [docs-infra] Add Vale [docs-infra] Enforce Vale Feb 13, 2024
@oliviertassinari oliviertassinari added enhancement This is not a bug, nor a new feature scope: docs-infra Specific to the docs-infra product and removed duplicate This issue or pull request already exists labels Feb 13, 2024
@oliviertassinari oliviertassinari marked this as not a duplicate of mui/material-ui#32486 Feb 13, 2024
@oliviertassinari oliviertassinari transferred this issue from mui/material-ui Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is not a bug, nor a new feature scope: docs-infra Specific to the docs-infra product test
Projects
None yet
Development

No branches or pull requests

2 participants