mui-public icon indicating copy to clipboard operation
mui-public copied to clipboard

[docs-infra] Enforce Vale

Open oliviertassinari opened this issue 2 years ago • 1 comments

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 https://github.com/mui/toolpad/pull/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 https://github.com/errata-ai/vale-action/issues/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 avatar Sep 02 '23 21:09 oliviertassinari

I think @samuelsycamore had created the same issue a while back! :) 👉 https://github.com/mui/material-ui/issues/32486

danilo-leal avatar Sep 04 '23 14:09 danilo-leal

Happy to enforce it, but it should be decided by devex (@mapache-salvaje @alelthomas) if we ultimately want to do this and after we have it over the whole codebase.

mnajdova avatar Oct 31 '25 13:10 mnajdova

I thought we already were enforcing Vale across the org?

mapache-salvaje avatar Oct 31 '25 14:10 mapache-salvaje

I thought we already were enforcing Vale across the org?

Vale only fails the CI in http://github.com/mui/material-ui. In the other repositories, we have to clean up the master branch every now and then, as people merge breakage.

oliviertassinari avatar Oct 31 '25 21:10 oliviertassinari