-
-
Notifications
You must be signed in to change notification settings - Fork 185
feat: enable auto-publishing on NPM after a GitHub release #1051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
commit: |
.github/workflows/release.yml
Outdated
jobs: | ||
publish: | ||
name: Publish package from release tag | ||
if: startsWith(github.event.release.tag_name, 'v') # only semver-like tags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: I don't thibnk that semver required the v
, so its not really a good check for sem-ver-ness, right?
instead, can we use a mechanism like CODEOWNERS
to restrict who can do releases, and then trust them to do it right?
source: https://semver.org/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The npm version
command produces a tag in the format of vX.Y.Z
and I want to rely on the standard practices.
instead, can we use a mechanism like CODEOWNERS to restrict who can do releases, and then trust them to do it right?
I believe that the default release rights already prohibit non-owners from creating releases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine for me to rely on the current auth-approach then.
The npm version command produces a tag in the format of vX.Y.Z and I want to rely on the standard practices.
I think we should either be relying on the gh-tagname OR on the package.json name + version. In the current logic you take the tag from github and the name from the package.json:
TAG="${{ github.event.release.tag_name }}"
NAME=$(node -p "require('./package.json').name")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that NAME
is only a debug variable, we might as well not use it at all. There is also a CI step that checks that the tag and the package json version match each other so that we prevent situations like manual tags created without bumping the package json version (or release being created too late/too early).
In this sense, we are solely relying on Git tags (because they serve as "code snapshots"), while only verifying package.json for correctness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you make the code a bit clearer/more easy to follow, so that people can better understand this when reading it? (:
|
||
- uses: pnpm/action-setup@v4 | ||
name: Install pnpm | ||
with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we specify the exact version of pnpm we want to use, to ensure consistent/stable builds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PNPM is already using a correct version, see: https://github.com/sidebase/nuxt-auth/actions/runs/17260489231/job/48980590409
Another piece of evidence:
- before: we had version
9.6.0
, it was correctly installed - after: I bumped to
10.15.0
, it was also correctly picked up
This means that the presence of packageManager
field inside package.json
is beneficial for both Corepack flow (keep the version consistent across contributors) and CI (use single source of truth) π
I checked the action source and it is in fact true that the action reads the version (also documented): https://github.com/pnpm/action-setup/blob/f2b2b233b538f500472c7274c7012f57857d8ce0/src/install-pnpm/run.ts#L54-L56
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Carrying over approval after successful dry-run.
After merging this I suggest to create a proper 1.0.2 release to see the whole thing in action (:
π Linked issue
β Type of change
π Description
This creates a workflow which would automatically publish the package on NPM when a GitHub (pre-)release has been published.
The full process of releasing would include:
pnpm version [patch|minor]
git push --follow-tags
π Checklist