Skip to content

Align lint/format/test config with the typescript-action template - #492

Merged
JKRT merged 4 commits into
OpenModelica:mainfrom
SVAGEN26:issue-465-align-with-typescript-action-template
Sep 9, 2026
Merged

JKRT merged 4 commits into
OpenModelica:mainfrom
SVAGEN26:issue-465-align-with-typescript-action-template

Conversation

@SVAGEN26

@SVAGEN26 SVAGEN26 commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Closes #465.

Starting point

The dependency half of #465 is already done — dependabot moved the repo onto the
ESM-only majors (@actions/core 3, @actions/exec 3, @actions/tool-cache 4,
@actions/cache 6), and "type": "module" + rollup + flat ESLint config are all
in place. What never caught up is the surrounding config — and most of it turned
out not to be exercised at all.

What was actually wrong

__tests__ was never linted. lint was eslint src/**/*.ts, so the
eslint-plugin-jest setup and the tsconfig.test.json entry in
parserOptions.project were dead weight. Now eslint . covers the tree. That
needs the root config files to belong to a project, so parserOptions.project
is replaced by the template's projectService + allowDefaultProject, and
coverage/ joins dist/ in the ignores.

eslint.config.mjs had never been checked by its own rules. Once linted it
reported an unused js import and an unused __filename/__dirname pair left
over from the flat-config migration. Removed, along with the node:path and
node:url imports that only fed them.

rollup.config.ts imported nodeResolve as a default import, which
import/no-named-as-default flags as soon as the file is linted. Switched to the
named import.

Jest config was doubled up. preset was the deprecated
ts-jest/presets/default-esm while transform already passed useESM: true,
and moduleNameMapper re-implemented what ts-jest-resolver was already doing.
Reduced to the template's preset: 'ts-jest' + resolver, plus its reporters.

tsconfig now excludes __tests__ and dist as the template does.
tsconfig.test.json declares its own exclude, so tests are still type-checked
there — tsc --noEmit -p tsconfig.test.json passes.

Nothing enforced any of it. test.yml ran only package and test, never
lint or format, so all of the above could drift indefinitely. Both are now steps
in the build job.

Scripts move to the template's format:write / format:check names and run over
the whole tree instead of **/*.ts.

Two commits

  1. the config change
  2. the one-time prettier pass over the Markdown and YAML that format:check now
    covers — kept separate because it is 118 lines of pure churn. git diff -w
    reduces it to prettier's own singleQuote preference and Markdown table
    padding; no content changes.

Verification

  • npm run package produces a byte-identical dist/index.js — git status dist/ is clean, so check-dist is unaffected and the shipped action does not
    change.
  • npm run lint, npm run format:check and tsc --noEmit -p tsconfig.test.json
    all pass.
  • npm test reports the same results as before the change (the failures are
    environmental — the suite really does apt install omc — and are identical on
    main).

Deliberately not done

eslint-plugin-github and @stylistic are kept rather than swapped for the
template's prettier-plugin-based ESLint config. The issue asks for the ESLint
configuration to resemble the template, but that swap changes which rules apply
to src/, which is a behavioural change to the action source rather than config
alignment. Happy to do it as a follow-up if you want the full template ruleset.

Also noticed while running the suite: the tests leave installLibs.mos and
linux-64.tar.gz in the working tree and neither is gitignored. Left alone here.

🤖 Generated with Claude Code

JKRT and others added 2 commits September 8, 2026 19:18
Closes OpenModelica#465.

The @actions packages are already on their ESM-only majors (core 3,
exec 3, tool-cache 4, cache 6) via dependabot, but the surrounding
config never caught up with the template, and most of it was not
actually being exercised.

- eslint: `eslint src/**/*.ts` only ever linted `src`, so the jest
  plugin config and `tsconfig.test.json` in `parserOptions.project`
  were dead weight -- `__tests__` was never linted. Lint the whole
  tree with `eslint .` instead. That requires the root config files to
  belong to a project, so swap `parserOptions.project` for the
  template's `projectService` + `allowDefaultProject`, and ignore
  `coverage/` alongside `dist/`.

- eslint.config.mjs had never been linted by its own rules: it kept
  `js` and a `__filename`/`__dirname` pair left over from the flat
  config migration, none of them used. Removed, along with the
  now-unused `node:path` and `node:url` imports.

- rollup.config.ts imported `nodeResolve` as a default import, which
  `import/no-named-as-default` flags once the file is linted. Use the
  named import.

- jest: `preset` was the deprecated `ts-jest/presets/default-esm`
  while `transform` already set `useESM`, and `moduleNameMapper`
  duplicated what `ts-jest-resolver` does. Reduce to the template's
  `preset: 'ts-jest'` + resolver, and add its `reporters`.

- tsconfig: exclude `__tests__` and `dist` as the template does.
  `tsconfig.test.json` declares its own `exclude`, so tests are still
  type-checked there.

- scripts: use the template's `format:write` / `format:check` names and
  run prettier over the whole tree rather than only `**/*.ts`.

- CI ran neither lint nor format, so none of the above was enforced.
  Add both to the build job.

`npm run package` produces a byte-identical `dist/index.js`, and the
test suite reports the same results as before the change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`format:check` runs over the whole tree rather than only `**/*.ts`, so
the Markdown and YAML that prettier had never touched need formatting
once for CI to pass.

Whitespace, quote style and Markdown table separators only -- `git diff
-w` reduces this to prettier's own `singleQuote` preference and table
padding, with no content change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@SVAGEN26

SVAGEN26 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@AnHeuermann review please — this closes #465, which you assigned to @JKRT.

(Claude Code agent acting on behalf of @JKRT.)

Two things worth knowing before you look:

  1. The dependency half of the issue was already done. Dependabot had
    already moved us to the ESM-only majors (@actions/core 3, exec 3,
    tool-cache 4, cache 6). What was left was the config around them — and
    most of it turned out not to be exercised: lint only ever covered src,
    so __tests__ was never linted, and CI ran neither lint nor format, so
    nothing enforced any of it.

  2. CI has not run. All four workflows are sitting at action_required
    because SVAGEN26 is a first-time contributor to this repo. If you approve
    the runs, build-test and check-dist should both go green — locally
    npm run package produces a byte-identical dist/index.js, so the shipped
    action is unchanged.

The second commit is a one-time prettier pass over the Markdown and YAML that
format:check now covers. It looks big (118 lines) but git diff -w reduces it
to quote style and Markdown table padding — no content changes. Kept separate so
the config commit stays readable.

One thing I did not do, deliberately: the issue asks for the ESLint config to
match the template, but that would mean dropping eslint-plugin-github and
@stylistic for the template's prettier-plugin setup. That changes which rules
apply to src/, which is a behavioural change to the action source rather than
config alignment, so I left it. Say the word if you want the full template
ruleset and I will do it as a follow-up.

`format:check` now covers the whole tree and runs in CI, which surfaced a
latent problem: on windows-latest git checks out CRLF, prettier expects LF,
and all 23 files fail. Neither condition held before -- the check was
scoped to `**/*.ts` and CI never ran it -- so this never showed up.

Add the template's `* text=auto eol=lf` so working trees are LF regardless
of platform.

Co-Authored-By: JKRT <jtinnerholm@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JKRT
JKRT enabled auto-merge (squash) September 8, 2026 18:16
auto-merge was automatically disabled September 8, 2026 18:54

Head branch was pushed to by a user without write access

@SVAGEN26
SVAGEN26 force-pushed the issue-465-align-with-typescript-action-template branch from 1a4f9bf to fea339d Compare September 8, 2026 18:54

@AnHeuermann AnHeuermann left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The apt release channel now serves OpenModelica 1.27.0, so the release
test failed against its 1.26 expectation. The last version bump moved
nightly and stable but missed this case.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JKRT
JKRT enabled auto-merge (squash) September 9, 2026 10:44
@JKRT
JKRT merged commit 784f231 into OpenModelica:main Sep 9, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Jest and TS config for upcoming @actions updates

3 participants