refactor(ci): improve workflows with centralized node version and github actions pinned to latest release versions#1022
Conversation
- Rename workflow to "Tests" - Split checks into parallel jobs (lint, typecheck, test, build) - All jobs depend on single 'install' job to cache npm - Use .nvmrc for Node version instead of hardcoding - Pin actions to commit SHA for supply chain security
Centralize Node version management by using node-version-file instead of hardcoding in each workflow.
Set minimum coverage thresholds matching current state: - statements: 1.73% - branches: 1.73% - functions: 0.88% - lines: 1.74% With failWhenThresholdNotMet, tests will fail if coverage drops.
…tions in all Github workflows
|
Thanks for trying to improve CI, but
This is 100% intentional. Linting is near instant, type checking is super fast, test is terribly slow. If a PR fails the first two steps, I don't wanna wait for the whole test suite to run! Also, if a test fails (which normally shouldn't happen because devs should test their changes locally), I wan't CI to fail fast rather than complete the full test suite. This actually speeds up iterations when working on a feature. I also find it strange that you pin workflow versions to a specific commit. |
I understand the intention. |
This may seem strange at first glance, given that version tags do exist. Recently, established distributed software development tools used by a large number of people have increasingly become targets of corruption by malware and other threats. These include widely used NPM packages and the most commonly used GitHub Actions (such as |
|
Huh interesting, thanks! |
There was a problem hiding this comment.
Pull request overview
Improves CI reliability and security by enforcing Vitest coverage thresholds, centralizing the Node.js version via .nvmrc, and pinning key GitHub Actions to commit SHAs.
Changes:
- Add Vitest coverage thresholds so CI fails on coverage regressions.
- Introduce
.nvmrcand update workflows to usenode-version-file. - Pin
actions/checkoutandactions/setup-nodeto SHAs for supply-chain hardening.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
vitest.config.js |
Enforces minimum coverage thresholds during vitest run --coverage. |
.nvmrc |
Defines the Node.js version consumed by CI workflows via node-version-file. |
.github/workflows/test.yml |
Uses .nvmrc, enables npm cache, and pins checkout/setup-node by SHA. |
.github/workflows/release.yml |
Uses .nvmrc and pins checkout/setup-node by SHA. |
.github/workflows/image-release.yml |
Uses .nvmrc and pins checkout/setup-node by SHA. |
.github/workflows/image-devel.yml |
Uses .nvmrc and pins checkout/setup-node by SHA. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Let's please remove the SHA pinning altogether. It's a maintenance burden to keep updating them manually which I don't want to have. |
The Dependabot configuration is taking care of Github Action Updates. When the related Dependabot PRs are opened and the workflows ran successful, the actions are updatable. |
|
The Github actions of all workflows are now pinned to the tags of their latest released versions. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Removed coverage thresholds from configuration.
Motivation
In the Github workflows
Changes
.nvmrcfile; all workflows now reference it vianode-version-fileinstead of hardcodingImpact
nvm useto set and use the exact Node.Js version configured in this repository