Azure / Azure/azure-rest-api-specs

Cleanup & reorganization follow-ups for pnpm workspace migration

Open
#44,284 0 comments 0 reactions 1 assignee Claimed by @timotheeguerin View on GitHub
Dominant language
TypeSpec
Stars
3.1k
Forks
5.9k
Avg merge
2d 22h
Merged PRs (30d)
444

Description

Follow-up cleanup for the pnpm workspace migration (#43922). That PR is intentionally
bare-minimum; the workspace model now lets us remove a lot of duplication and inconsistency
across `eng/tools/*`, `.github`, and `.github/shared`.

Each item below is scoped to its **own independent PR**. PRs 1–4 are largely independent;
PR 5 depends on 1–4; PR 6 depends on 1 (ideally 5). All changes are meant to be
behavior-preserving (same lint rules, same prettier output, same tests) — reorganization, not
a rules change.

## Background / current-state findings

- Root `package.json` has **no aggregate scripts** (only `install-pnpm`) — no workspace-level
`format` / `lint` / `build` / `test` / `check`.
- Every `eng/tools/*` package (plus `.github`, `.github/shared`) **copy-pastes identical
script boilerplate**, e.g. `prettier . --ignore-path ../.prettierignore ...`,
`cross-env DEBUG=... eslint`, `vitest run --coverage --reporter=verbose`.
- **Lint coverage gaps**: `openapi-diff-runner`, `sdk-suppressions`, `summarize-impact`,
`typespec-migration-validation` have no `lint`/`check` script and no `eslint.config.js` —
they are effectively **not linted today**.
- `eng/tools/.prettierrc.yaml` and `.github/.prettierrc.yaml` are byte-identical with a manual
"Keep in sync with..." comment; two `.prettierignore` files.
- `test` is defined three different ways across packages (`vitest`, `vitest run`, `vitest --run`).
- ~11 near-identical per-tool `*-test.yaml` workflows each call `_reusable-eng-tools-test.yaml`;
`github-test.yaml` is a separate near-duplicate for `.github` + `.github/shared`.

## Tasks (one PR each)

- [ ] **1. Add root aggregate scripts.** Add `format`, `format:check`, `format:check:ci`,
`lint`, `build`, `test`, `test:ci`, `check` to root `package.json` driven by `pnpm -r`.
Additive and non-breaking (existing per-package scripts stay).
_Acceptance:_ `pnpm check` at the repo root builds + lints + format-checks + tests every
workspace package.

- [ ] **2. Centralize prettier config.** One shared code-prettier config + one `.prettierignore`
covering `eng/tools` and `.github`/`.github/shared`; delete the duplicated
`eng/tools/.prettierrc.yaml` / `.github/.prettierrc.yaml` (and the "keep in sync" comment) and
remove the `--ignore-path ../.prettierignore` usages. **Leave the root `.prettierrc.json`
(swagger plugin, `printWidth: 20`) untouched** — that governs spec JSON and is a separate
concern from `Swagger-Prettier-Check.ps1`.
_Acceptance:_ `prettier --check` output is byte-identical to before.

- [ ] **3. Fix lint coverage gaps.** Add `eslint.config.js` (thin wrapper over
`eng/tools/eslint.base.config.js`) for `openapi-diff-runner`, `sdk-suppressions`,
`summarize-impact`, `typespec-migration-validation`, and fix any newly-surfaced lint errors.
_Acceptance:_ every `eng/tools/*` package is covered by lint.

- [ ] **4. Centralize eslint invocation.** Run eslint once from the root / `eng/tools` over all
packages (single config or thin wrappers), so lint isn't re-invoked per package.
_Acceptance:_ root `pnpm lint` lints the whole workspace; no rule changes.

- [ ] **5. Normalize + slim per-package scripts.** Remove `format*`, `format:check:ci`, and
`lint` from every `eng/tools/*`, `.github`, `.github/shared` package; keep only `build` and a
single normalized `test` / `test:ci`. _(Depends on 1–4.)_
_Acceptance:_ package `package.json`s are minimal; root scripts cover everything.

- [ ] **6. Consolidate CI workflows.** Replace the ~11 per-tool `*-test.yaml` +
`_reusable-eng-tools-test.yaml` (and fold in `github-test.yaml`) with a single
`eng-tools-check.yaml` that installs once and runs
`pnpm -r --filter "...[origin/main]"` so only **affected packages** build/test, plus the
centralized root `lint` + `format:check:ci`. Preserve the `.github`-specific steps (actionlint,
"all modules importable" check) gated to when `.github` is affected, and handle the
push-vs-PR base-ref selection (full run on `main`). _(Depends on 1; ideally 5.)_
_Acceptance:_ PRs run only changed packages; `main` runs the full set.

## Notes

- Keep everything behavior-preserving — same lint rules, same prettier output, same tests.
- Watch for references to package-level scripts in `eng/scripts/*.ps1` and
`eng/pipelines/.../npm-install.yml` (e.g. `format:check:ci`, `run lint`, `working-directory`)
and update any that break when scripts move to the root.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.