ci: bootstrap build/lint/test workflow and wire eslint - #6
Merged
Merged
Conversation
.eslintrc.js extended universe/native and universe/web, but neither eslint nor eslint-config-universe was ever a dependency, and eslint 9 ignores .eslintrc.* regardless. The config was vestigial and had never run against this repo. Adds eslint, eslint-config-universe and prettier as devDependencies, replaces .eslintrc.js with eslint.config.js, and adds a lint script gated at zero warnings. .prettierrc pins single quotes to match the existing source rather than reformatting every string in the repo to universe's double-quote default. That alone took the first run from 161 problems to 13. Of the remainder, nine were auto-fixable import order and wrapping. Two needed a decision, both suppressed at the line rather than in config: no-void, on the one `void endSigningSession()` in the repo. It is the idiom for a deliberately unawaited promise and reads better than the `undefined` the rule asks for. react-hooks/set-state-in-effect, on the auto-initialize effect. doInitialize sets state on its first line, which the rule reads as a cascading render. It is deliberate: that effect exists to bring the DocuSign SDK up, and the INITIALIZING transition is how the progress is reported. The cost is one extra render on mount. Reworking the hook's init lifecycle is a behaviour change and does not belong in the commit that turns lint on. The lockfile's root version was also stale at 1.0.3 against a package.json already at 1.0.5. Installing the new devDependencies corrects it, which npm ci would have required anyway.
The protect-main ruleset requires status checks named build, lint and test, and also CodeQL results. The repo had no workflows at all, so none of those were ever reported and every PR sat permanently blocked on checks that had no producer. #3 is currently stuck exactly this way. Job ids are build / lint / test deliberately: with no name override, the job id becomes the status check context, which is what the ruleset matches on. Renaming a job here silently makes its required check unsatisfiable again. Three separate jobs rather than one with three steps, so each context reports independently and a lint failure does not mask a test failure. permissions is contents: read, there are no secrets, and no untrusted context values reach any run step. Actions are tag-pinned rather than SHA-pinned, which is worth revisiting if the repo standardises on stricter supply-chain rules.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bootstraps CI. This PR needs the ruleset bypass to merge, and it is the only one that should.
Why
The
protect-mainruleset requires status checks namedbuild,lintandtest, plus CodeQL code scanning results. The repo had no.github/workflowsdirectory at all and CodeQL default setup reportsnot-configured, so none of those four have ever been reported by anything. Every PR is permanently blocked on checks with no producer. #3 is sitting in exactly that state right now with zero check runs on its head commit.That is a bootstrap deadlock: the CI that satisfies the rules has to land on
main, but landing onmainrequires satisfying the rules. Hence the one-time bypass. Once this merges, every subsequent PR gets real checks and no bypass is needed again.Lint had never actually run
.eslintrc.jsextendeduniverse/nativeanduniverse/web, but neithereslintnoreslint-config-universewas inpackage.json, and ESLint 9+ ignores.eslintrc.*regardless. The config was vestigial.Wiring it up surfaced 161 problems on the first run.
.prettierrcpinningsingleQuotetook that to 13, by matching the config to the source rather than reformatting every string in the repo to universe's double-quote default. Nine of the rest were auto-fixable import order and wrapping.Two needed a judgement call, both suppressed at the line rather than switched off in config:
no-voidon the singlevoid endSigningSession()in the repo. It is the idiom for a deliberately unawaited promise.react-hooks/set-state-in-effecton the auto-initialize effect.doInitializesets state on its first line, which the rule reads as a cascading render. It is deliberate: that effect brings the DocuSign SDK up, and theINITIALIZINGtransition reports its progress. Cost is one extra render on mount. Reworking the hook's init lifecycle is a behaviour change and does not belong in the commit that turns lint on.Notes
Job ids are
build/lint/testdeliberately. With noname:override the job id becomes the status check context, which is what the ruleset matches on. Renaming a job silently makes its required check unsatisfiable again.The lockfile's root
versionwas stale at1.0.3against apackage.jsonalready at1.0.5. Installing the new devDependencies corrects it.npm ciwould have required that anyway, so it is a prerequisite rather than scope creep. Not a version bump.CodeQL default setup is enabled separately via the API, since it is repo configuration rather than a file in the tree.
Verification
npm ci && npm run build && npm run lint && npm testall green locally. 13 tests passing.