Azure / Azure/azure-rest-api-specs
Cleanup & reorganization follow-ups for pnpm workspace migration
- 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
Assessment
This issue has not been assessed yet.