Repository navigation
Upgrade GitHub Actions, refactor CI setup, align Node engines - #23
Conversation
ame-axelor
left a comment
There was a problem hiding this comment.
The upgraded jobs look correct — step reordering for cache: "pnpm" is right, and dropping version: latest is fine since package.json pins packageManager: pnpm@10.19.0. CI is green on build/test/test-e2e. A few things to address before merge though:
-
Partial upgrade — please finish the job. The PR title says "v6" but
publish-nextandpublishare still onactions/checkout@v3,actions/setup-node@v3, andpnpm/action-setup@v3(lines ~71-80 and ~111-118 of.github/workflows/gradle.yml). Onlynode-versionwas bumped there. Two problems:- The file now has the same setup pattern in two different versions side-by-side — confusing for future readers.
- Those jobs only run on
pushtodevor on tag push, so they show asskippingin this PR's checks. The v3-actions-with-Node-22 combo is not actually exercised by CI here, and any incompatibility will only surface after merge.
Please upgrade them to
@v6and addcache: "pnpm"for consistency. -
engines.nodemismatch.package.jsonstill declares"node": ">=18.18.0"while CI now only tests on Node 22. Either keep Node 18 in the matrix or bumpengines.nodeso consumers don't install on a version we no longer test. -
(Optional)
actions/create-release@v1is archived (line ~105). Not introduced by this PR but right next to lines you're touching — worth replacing withsoftprops/action-gh-releaseor agh release createstep in a follow-up. -
(Optional) DRY the setup. All five jobs duplicate the same
checkout + setup-node + pnpm-setup + installblock. A composite action under.github/actions/setup/would collapse them and prevent partial-upgrade asymmetry like the one above. Out of scope for this PR, but if you're touching every job anyway, it's a cheap follow-up.
Items 1 and 2 are the blockers; 3 and 4 are nice-to-haves. Happy to approve once the publish jobs are aligned.
…d create-release - actions/checkout: v3 -> v6 - actions/setup-node: v3 -> v6 (node 18 -> 22) - pnpm/action-setup: v3 -> v6 - docker/setup-docker-action: v4 -> v5 - Replace actions/create-release@v1 (deprecated) with softprops/action-gh-release@v3 - Extract pnpm/Node setup into .github/actions/setup composite action
Align with CI (Node 22) and pin to first LTS release of the v22 line.
4f673df to
3f47f32
Compare
|
All issues addressed. Please let me know if further changes required |
ame-axelor
left a comment
There was a problem hiding this comment.
All blockers addressed and both optional follow-ups landed too — composite action extracted, actions/create-release swapped for softprops/action-gh-release, engines.node aligned, plus docker/setup-docker-action bumped. CI is green on build/test/test-e2e.
Note for after merge: the engines bump to >=22.11.0 is consumer-visible — worth flagging in the changelog and bumping the package minor. The new release flow (npm publish → GitHub release via softprops) will be exercised for the first time on the next tag push; worth a watchful eye.
Summary
Upgrade all GitHub Actions to their latest majors, deduplicate the pnpm/Node setup into a composite action, replace a deprecated action, and align
engines.nodewith what CI actually exercises.Changes
Action upgrades
actions/checkout: v3 → v6actions/setup-node: v3 → v6 (Node 18 → 22.x)pnpm/action-setup: v3 → v6 (dropsversion: latest; reads pnpm version frompackageManagerinpackage.json)docker/setup-docker-action: v4 → v5actions/create-release@v1(archived/deprecated) →softprops/action-gh-release@v3Workflow refactor
.github/actions/setupcomposite action. Avoids the partial-upgrade asymmetry flagged in earlier review and removes 4× duplication.permissions: contents: writeon the release job (required bysoftprops/action-gh-release).Engines
engines.nodefrom>=18.18.0to>=22.11.0so the declared minimum matches CI.22.11.0is the first LTS release of the v22 line.Test plan
build,test,test-e2ejobs green on this PRpublish-nextruns cleanly on merge todevpublishruns cleanly on next tag push (release creation now viasoftprops/action-gh-release)