Skip to content

Commit 4ee366f

Browse files
committed
Add pull request CI and a working lint setup
Closes #46. The repository had no CI for pull requests. Only publish.yml existed, triggered by tags, so a change was first executed by a machine other than the author's at release time. #48 is a 908-line contribution from outside, and reviewing it without an automated build was the immediate reason to fix this. The new workflow runs on pull requests and on pushes to main: npm ci, lint, build, test, then a check that the build actually produced something. `npm ci` rather than `npm install`, so a pull request whose lockfile disagrees with package.json fails instead of silently resolving something else. The matrix is Node 18 and 24. 18 is the floor declared in engines, 24 is what publish.yml releases with. Testing only one of them would let through a release that satisfies neither its own engines field nor its own publish path. The final step verifies dist/cli.js, dist/mcp.js, dist/remote-helper.js and dist/index.js are non-empty and that `node dist/cli.js --version` runs. tsc exiting zero does not prove entry points were emitted, and dist is the entire published package. test/e2e*.sh are deliberately not run. They drive a real Overleaf account through login, pull, push and compile against a live project. `npm test` globs test/*.test.ts, so the shell suites are already outside it, and the workflow says so in a comment to keep someone from widening the glob later. eslint.config.js is flat config for eslint 9, with typescript-eslint recommended and without type-checked rules. Those need a TypeScript program per run and would turn lint into a second type checker with its own opinions; tsc already runs in CI and is the authority there. `no-explicit-any` is a warning rather than an error, and the count is the reason. 58 occurrences remain, all pre-existing, concentrated in client.ts and cli.ts where untyped JSON comes back from Overleaf. Overleaf publishes no schema for those responses, so each one is a typing decision rather than a mechanical fix. As an error, CI would be red on main from the day it is switched on, which teaches everyone to ignore it. As a warning it stays visible and blocks nothing. Worth revisiting once the count is small enough to clear in one pass. Verified locally: lint, build and test each exit zero, and the workflow parses as YAML.
1 parent 8f02339 commit 4ee366f

4 files changed

Lines changed: 1481 additions & 2 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
name: CI
2+
3+
# Runs on pull requests and on pushes to main. Nothing here publishes; the
4+
# release path stays in publish.yml, which is triggered by tags only.
5+
#
6+
# ⚠️ test/e2e*.sh are deliberately NOT run here. They drive a real Overleaf
7+
# account: login, pull, push, compile against a live project. `npm test` globs
8+
# `test/*.test.ts`, so the shell suites are already outside it - do not "fix"
9+
# that by widening the glob.
10+
on:
11+
pull_request:
12+
push:
13+
branches: [main]
14+
15+
permissions:
16+
contents: read
17+
18+
concurrency:
19+
# A second push to the same PR cancels the first; no point finishing a run
20+
# for a commit nobody will merge.
21+
group: ci-${{ github.workflow }}-${{ github.ref }}
22+
cancel-in-progress: true
23+
24+
jobs:
25+
build:
26+
runs-on: ubuntu-latest
27+
strategy:
28+
# One failing version should not hide the others.
29+
fail-fast: false
30+
matrix:
31+
# 18 is the floor declared in package.json engines, 24 is what
32+
# publish.yml ships with. Both are tested because a release that only
33+
# works on the newer one would still satisfy the engines field.
34+
node-version: ['18', '24']
35+
36+
steps:
37+
- uses: actions/checkout@v4
38+
39+
- uses: actions/setup-node@v4
40+
with:
41+
node-version: ${{ matrix.node-version }}
42+
cache: npm
43+
44+
# ci, not install: fails if package-lock.json and package.json disagree,
45+
# which is exactly what should stop a pull request.
46+
- run: npm ci
47+
48+
- name: Lint
49+
run: npm run lint
50+
51+
- name: Build
52+
run: npm run build
53+
54+
- name: Test
55+
run: npm test
56+
57+
# The published package is dist/ plus README and LICENSE. A build that
58+
# compiles but produces no entry points would otherwise pass everything
59+
# above and still ship broken.
60+
- name: Verify build output
61+
run: |
62+
for f in dist/cli.js dist/mcp.js dist/remote-helper.js dist/index.js; do
63+
test -s "$f" || { echo "missing or empty: $f"; exit 1; }
64+
done
65+
node dist/cli.js --version

‎eslint.config.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// ESLint flat config (ESLint 9). The package is ESM, so this file is too.
2+
//
3+
// Scope is deliberately narrow: this exists so `npm run lint` has a working
4+
// setup behind it and so CI can run it on pull requests. It is not an attempt
5+
// to restyle the codebase.
6+
//
7+
// Type-checked rules are NOT enabled. They need a TypeScript program per run,
8+
// which is slower and, more to the point, would turn a lint job into a second
9+
// compiler with its own opinions. `tsc` already runs in CI and is the
10+
// authority on types.
11+
import js from "@eslint/js";
12+
import tseslint from "typescript-eslint";
13+
14+
export default tseslint.config(
15+
{
16+
// dist/ is build output, node_modules/ is not ours. Neither is source.
17+
ignores: ["dist/**", "node_modules/**", "coverage/**"],
18+
},
19+
js.configs.recommended,
20+
...tseslint.configs.recommended,
21+
{
22+
files: ["src/**/*.ts"],
23+
rules: {
24+
// `_`-prefixed arguments are the established way to say "required by the
25+
// signature, unused on purpose" - commander callbacks and catch blocks
26+
// both hit this.
27+
"@typescript-eslint/no-unused-vars": [
28+
"error",
29+
{
30+
argsIgnorePattern: "^_",
31+
varsIgnorePattern: "^_",
32+
caughtErrorsIgnorePattern: "^_",
33+
},
34+
],
35+
36+
// Warning, not error, and the number is the reason: 58 occurrences in
37+
// src/client.ts and src/cli.ts, all of them pre-existing. They sit where
38+
// untyped JSON comes back from Overleaf, which has no published schema,
39+
// so each one is a real typing decision rather than a mechanical fix.
40+
//
41+
// Making this an error would mean CI fails on `main` from the day it is
42+
// switched on, which trains everyone to ignore it. As a warning it stays
43+
// visible and does not block, and new code can be held to a higher bar in
44+
// review. Revisit once the count is low enough to fix in one pass.
45+
"@typescript-eslint/no-explicit-any": "warn",
46+
},
47+
},
48+
);

0 commit comments

Comments
 (0)