Skip to content
Merged
Changes from all commits
Commits
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
35 changes: 35 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Managed by sh1pt Actions Fleet
# pack: node-pnpm-test@1.0.0
# install: sh1pt-actions-store
# hash: sha256:14a3f6fdbc21be92e815a16317251c967e4fbd95a5daf98819e0c23e7d56bf4f
name: test

on:
pull_request:
push:
branches: [master]
Comment on lines +6 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Overlapping triggers with existing ci.yml

The existing .github/workflows/ci.yml already runs pnpm install --frozen-lockfile and pnpm test on every pull_request and on push to master. This new workflow duplicates those exact steps for the same trigger events, so every PR and every master push will now run pnpm test twice in parallel, consuming double the CI minutes with no additional coverage.


permissions:
contents: read

jobs:
test:
runs-on: ubuntu-latest
timeout-minutes: 15
Comment on lines +15 to +18

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Missing concurrency group

The existing ci.yml cancels in-progress runs with a concurrency group, so redundant queued runs are cleaned up automatically. Without this, rapid pushes or frequent PR updates will stack up multiple test workflow runs simultaneously.

Suggested change
jobs:
test:
runs-on: ubuntu-latest
timeout-minutes: 15
concurrency:
group: test-${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
jobs:
test:
runs-on: ubuntu-latest
timeout-minutes: 15

steps:
- uses: actions/checkout@v4

- uses: pnpm/action-setup@v4
with:
version: 9.12.0
Comment on lines +22 to +24

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Inconsistent pnpm version with ci.yml

ci.yml uses version: 9 (tracks the latest 9.x patch automatically), while this workflow hard-pins to 9.12.0. When pnpm 9.x releases bug or security fixes, ci.yml picks them up but test.yml stays behind, making the two environments subtly differ.

Suggested change
- uses: pnpm/action-setup@v4
with:
version: 9.12.0
- uses: pnpm/action-setup@v4
with:
version: 9

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


- uses: actions/setup-node@v4
with:
node-version: 22
cache: pnpm

- run: pnpm install --frozen-lockfile

- run: pnpm test
env:
CI: true
Loading