Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 7 additions & 39 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,20 @@ on:
pull_request:
paths-ignore:
- "**.md"
- dist/**
- __tests__/dotnet/**
push:
branches:
- main
paths-ignore:
- "**.md"
- dist/**
- __tests__/dotnet/**

jobs:
build:
name: Build
check:
name: Lint & Test
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.ref || github.ref_name }}

- name: Set up pnpm
uses: pnpm/action-setup@v6
Expand All @@ -34,37 +31,8 @@ jobs:

- run: pnpm install

- run: |
pnpm all

- name: Compare the expected and actual dist/ directories
id: diff
run: |
if [ "$(git diff --ignore-space-at-eol dist/ | wc -l)" -gt "0" ]; then
echo "Detected uncommitted changes after build. See status below:"
git diff
echo "diff=true" >> $GITHUB_ENV
else
echo "diff=false" >> $GITHUB_ENV
fi
- name: Lint & format check
run: pnpm run format-check

- name: commit build
id: commit
if: ${{ env.diff == 'true' }}
run: |
git config --local user.email "maxisam@gmail.com"
git config --local user.name "maxisam"
git add dist/
git commit -m "chore: build, dist updated"
git push origin ${{ github.event.pull_request.head.ref || github.ref_name }}
sha=$(git rev-parse HEAD)
echo "sha=$sha" >> $GITHUB_OUTPUT

- name: Status update
if: ${{ env.diff == 'true' }}
uses: myrotvorets/set-commit-status-action@v2.0.0
with:
status: "success"
sha: ${{ steps.commit.outputs.sha }}
description: "Build Dist Updated"
context: "Build Dist Updated"
- name: Test
run: pnpm test
131 changes: 131 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
# Migration Plan — Bundled JS Action → Hybrid Composite Action

## Goal

Replace the bundled TypeScript GitHub Action (compiled with `ncc` into a committed
`dist/index.js`) with a **hybrid composite action**. The win we're after is killing
the build step and the committed bundle, shrinking the dependency surface, and making
what runs match what you read in `action.yml`.

## Why hybrid (not pure shell)

Most of the action is orchestration around two CLIs (`dotnet format`, `jscpd`) plus
GitHub plumbing — all of which is *simpler* as composite steps. But two parts are
error-prone in bash:

1. **Config merging** — a 3-way deep merge (code defaults → root config → workspace
config) over JSON **and** YAML, with array de-duplication and the
`isEnabled ?? isEabled ?? default` fallback plus simple-vs-granular precedence.
2. **`--include`/`--exclude` list building** — space-joined argument lists are a
classic bash quoting hazard.

Those two stay in small, unit-tested JavaScript helpers. Everything else becomes
shell or [`actions/github-script`](https://github.com/actions/github-script).

## Three kinds of steps

| Kind | Used for |
|---|---|
| **shell** | run the CLIs: `dotnet format`, `jscpd`, `git`, nuget restore |
| **`actions/github-script`** | anything touching the GitHub API or `@actions/core`: changed-files lookup, PR comments, job summary, outputs, annotations, problem matcher |
| **tested `scripts/*.mjs` helpers** | the two error-prone pure-logic bits (config merge, report→markdown), `require()`d from a github-script step so they stay unit-testable |

`actions/github-script` hands you a pre-authenticated `github` (octokit), `context`,
`core`, `glob`, `io`, and `exec` with **no build and no committed bundle** — removing
almost the entire reason `dist/` exists today.

## Architecture / step sequence

`action.yml` becomes `runs.using: "composite"`, same `inputs` and `outputs`. Data flows
between steps via a normalized JSON file in `$RUNNER_TEMP` (resolved config + ready-to-run
arg arrays) and small step outputs for scalars.

> **Gotcha baked in everywhere:** composite steps don't auto-expose `inputs` to the
> step body. Every shell/github-script step receives what it needs via `env:`, and
> github-script auth uses `github-token: ${{ inputs.authToken }}`.

1. setup env + `::add-matcher::` (shell)
2. resolve config → `$RUNNER_TEMP/df-config.json` (github-script → `read-config.mjs` + `format-args.mjs`)
3. changed files, if `onlyChangedFiles` + PR event (github-script)
4. `dotnet format` per enabled block (shell, consumes config json)
5. format report → markdown → summary + PR comment (github-script → `dotnet-report.mjs`)
6. `actions/upload-artifact@v7.0.1` (dotnet report)
7. commit & push with rebase-retry + `hasChanges` output (shell)
8. `::remove-matcher::` (shell)
9. jscpd run (shell, PATH-or-`npx jscpd@5`)
10. jscpd report → threshold → annotations → comment → summary + `hasDuplicates` (github-script → `jscpd-report.mjs`)
11. `actions/upload-artifact@v7.0.1` (jscpd report)
12. `failFast` gate (shell)

Note: artifact upload is the one thing github-script can't do (no `@actions/artifact`
bundled), so it uses the standard `upload-artifact` action — simpler than the current
`DefaultArtifactClient` code anyway.

## Task list (commit after each)

### Phase 0 — scaffold
- **T0**: Worktree `../dotnet-format-plus-composite` on branch `feat/composite-action`. ✅

### Phase 1 — tested helpers (port logic 1:1) ✅
- **T1** ✅: `scripts/merge.mjs` (deep-merge + dedupe, replaces `deepmerge`) +
`scripts/read-config.mjs` — port `readConfig` 3-way merge (defaults → root → workspace),
JSON+YAML (parser injected), array-dedup. `isEnabled ?? isEabled ?? default` +
simple-vs-granular precedence live in `format-args.mjs`.
- **T2** ✅: `scripts/format-args.mjs` — port `buildArgs` (esp. `--include`/`--exclude`
list joining) so the shell step never does bash quoting, plus `buildDefaultOptions`,
`finalizeEnabled`, `checkIsDryRun`, and a `planFormat` convenience emitting ready-to-run
`dotnet` argv arrays + `isDryRun`.
- **T3** ✅: `scripts/report-common.mjs` (shared footer) + `scripts/dotnet-report.mjs` +
`scripts/jscpd-report.mjs` — port JSON→markdown with the GitHub blob links (context
injected, not read from `@actions/github`).
- **T4** ✅: Ported the existing `node:test` suite onto these helpers (config-merge
/ arg / report coverage). YAML via inline `js-yaml` parse or `npx -y js-yaml` conversion —
no repo `node_modules` needed at action runtime.

### Phase 2 — composite skeleton ✅
- **T5** ✅: Rewrite `action.yml` → composite, same inputs/outputs; ship `problem-matcher.json`
at action root; matcher add/remove via workflow commands.

### Phase 3 — dotnet format path ✅
- **T6** ✅: config-resolve step → temp JSON.
- **T7** ✅: changed-files github-script step.
- **T8** ✅: format runner (shell loop over blocks).
- **T9** ✅: report→summary→comment github-script step (reuse existing-comment-by-header +
bot-user matching).
- **T10** ✅: `upload-artifact` (dotnet).
- **T11** ✅: commit/push shell step + `hasChanges`.

### Phase 4 — jscpd path ✅
- **T12** ✅: jscpd runner (shell).
- **T13** ✅: jscpd report github-script step (threshold, annotations, comment, summary,
`hasDuplicates`).
- **T14** ✅: `upload-artifact` (jscpd) plus cleanup after upload.

### Phase 5 — failFast + teardown of the build pipeline ✅
- **T15** ✅: `failFast` gate.
- **T16** ✅: Delete `dist/`, drop `package`/`ncc`/`finalize-dist`, remove `@actions/*`,
`@octokit/rest`, `deepmerge` from deps (keep only what helpers/tests need).
`package.json` shrinks to lint + test.
- **T17** ✅: Update CI: drop the dist-sync check; run biome + helper tests only.

### Phase 6 — docs & end-to-end ✅
- **T18** ✅: Rewrite README (no build step) + WALKTHROUGH for the composite migration.
- **T19** ✅: Local verification completed (`pnpm install --frozen-lockfile`, `pnpm run
format-check`, `pnpm test`, `pnpm all`, `git diff --check`, action metadata parse).
GitHub-hosted end-to-end confirmation remains the release gate: run
`test-dotnet-format.yml` (both fixture jobs) and verify PR comment, summary,
`hasChanges`/`hasDuplicates`, and annotations.

## Parity-risk checklist (things that can silently diverge)

- 3-way deep merge + array dedup + `isEabled` fallback (T1) — highest risk; the tests in
T4 exist to pin it.
- `--include` with changed-files list quoting — handled by keeping it in JS (T2).
- Existing-comment matching: header `startsWith` + `user.type === 'Bot'` / login (T9/T13).
- Non-PR events (push): comments skipped, summary still written.
- Outputs are exactly the strings `"true"` / `"false"`.

## Net result

No bundler, no committed `dist/`, dependency surface drops from 8 runtime deps to ~1,
and the only "real code" left is ~4 small tested helpers.
47 changes: 34 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,19 @@ Yet another dotnet format. It combines dotnet format with jscpd to provide a sin

## Requirements

- **Node 24** runtime — the action runs on `node24` (handled automatically by GitHub).
This is a **composite action** — it orchestrates shell steps and
[`actions/github-script`](https://github.com/actions/github-script), so there is no
bundled JavaScript to build or ship.

- **.NET SDK** must be available on the runner (e.g. via `actions/setup-dotnet`). The
bundled example/tests target **.NET 10**.
included example fixtures target **.NET 10**.
- **GitHub-hosted runners** provide everything else it needs (`bash`, `jq`, `git`,
Node/`npx`). On self-hosted runners make sure those are present.
- **jscpd 5** — when `jscpdCheck` is enabled, the action uses a `jscpd`/`cpd` binary found
on `PATH`, otherwise it fetches it on demand with `npx --yes jscpd@5` (a one-time download).
To avoid the download, install jscpd on the runner (`npm i -g jscpd`).
- **YAML config** (`.dotnet-format.yml` / `.jscpd.yml`) is parsed on demand via
`npx -y js-yaml`; JSON config needs nothing extra.

> Note on config keys: the granular `options`/`styleOptions`/`analyzersOptions`/`whitespaceOptions`
> blocks are toggled with `isEnabled`. The historical misspelling `isEabled` is still accepted for
Expand Down Expand Up @@ -53,25 +60,39 @@ example:

## Development

This is a TypeScript GitHub Action bundled with [`ncc`](https://github.com/vercel/ncc) into
`dist/`. The toolchain is modern and dependency-light:
This is a **composite action**: [`action.yml`](./action.yml) wires together shell steps
(which run `dotnet format`, `jscpd`, and `git`) and
[`actions/github-script`](https://github.com/actions/github-script) steps (which call the
GitHub API and `@actions/core`). The error-prone pure logic — config merge, `dotnet format`
argument building, and report→markdown — lives in small, dependency-free ES modules under
`scripts/` that the github-script steps load with a dynamic `import()`. **There is no build
step and nothing is bundled or committed to `dist/`.**

Toolchain:

- **Node 24** (ESM, `"type": "module"`)
- **[pnpm](https://pnpm.io/)** as the package manager (`packageManager` field; CI uses `pnpm/action-setup`)
- **[Biome](https://biomejs.dev/)** for linting + formatting (`biome.json`)
- **`node:test`** (Node's built-in runner) for tests — no Jest
- **`node:test`** (Node's built-in runner) for the helper unit tests — no Jest, no transpile

```bash
pnpm install
pnpm build # tsc --noEmit typecheck
pnpm lint # biome lint
pnpm test # node --test
pnpm package # ncc build -> dist/index.js
pnpm all # everything CI runs (build, format-check, package, test, finalize dist)
pnpm lint # biome lint
pnpm format-check # biome check (lint + format + import order)
pnpm test # node --test (unit tests for scripts/)
pnpm all # everything CI runs (biome check + node --test)
```

The committed `dist/` must stay in sync with the source — run `pnpm all` and commit `dist/`
before pushing. See [WALKTHROUGH.md](./WALKTHROUGH.md) for the design rationale and migration notes.
Layout:

- `scripts/*.mjs` — pure, dependency-free helpers (merge, config read, format-args, report markdown), unit-tested
- `scripts/steps/*.mjs` — thin github-script wrappers (resolve-config, format-report, jscpd-report, comment)
- `__tests__/*.test.mjs` — `node:test` coverage for the pure helpers
- `__tests__/dotnet/**` — `.NET 10` fixtures for the end-to-end workflow
- `problem-matcher.json` — referenced by `action.yml` via `${{ github.action_path }}`

End-to-end behavior is exercised by `.github/workflows/test-dotnet-format.yml` against the
`net10.0` fixtures. See [WALKTHROUGH.md](./WALKTHROUGH.md) for the design rationale and
[MIGRATION.md](./MIGRATION.md) for the migration plan.

## Acknowledgements

Expand Down
Loading
Loading