-
Notifications
You must be signed in to change notification settings - Fork 4
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
Adds npm token config to the automatic release process #103
base: main
Are you sure you want to change the base?
Conversation
@@ -11,6 +11,9 @@ on: | |||
GITHUB_USER_TOKEN: | |||
description: Authentication token with write permission needed by the release bot (falls back to GITHUB_TOKEN). | |||
required: false | |||
NPM_TOKEN: |
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.
Isn't that the same as the NPM_REGISTRY_TOKEN
- see for example https://github.com/inpsyde/reusable-workflows/blob/main/.github/workflows/wp-scripts-lint.yml#L56-L58 which is put into NODE_AUTH_TOKEN: ${{ secrets.NPM_REGISTRY_TOKEN }}
- see https://github.com/inpsyde/reusable-workflows/blob/main/.github/workflows/wp-scripts-lint.yml#L78 ?
The Github action "setup-node" will automatically read this token from env.NODE_AUTH_TOKEN
and set it.
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.
It is aligned to use the NPM_REGISTRY_TOKEN
secret and internally we use it to set NPM_TOKEN
since this is what it is consumed by the underlying implementation see: https://github.com/semantic-release/semantic-release/blob/master/docs/usage/ci-configuration.md#authentication-for-plugins
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.
That's fine if NPM_TOKEN
is used for semantic-release, but we always name our inputs/secrets the same. The input should be named NPM_REGISTRY_TOKEN
, how you use it internally is up to you or the consuming action/step.
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 align everything npm token-related to the other workflows and documentations?
Please ensure to update the documentation afterward (e.g., new table for inputs, etc.). Thanks!
GITHUB_USER_TOKEN: | ||
description: Authentication token with write permission needed by the release bot (falls back to GITHUB_TOKEN). | ||
required: false | ||
NPM_REGISTRY_TOKEN: | ||
description: Authentication token with write permission needed by NPM to release a package (falls back to GITHUB_TOKEN). | ||
required: false |
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 move this above GITHUB_USER_TOKEN
to align with other workflows that put it above any GITHUB_*
secret?
GITHUB_USER_TOKEN: | |
description: Authentication token with write permission needed by the release bot (falls back to GITHUB_TOKEN). | |
required: false | |
NPM_REGISTRY_TOKEN: | |
description: Authentication token with write permission needed by NPM to release a package (falls back to GITHUB_TOKEN). | |
required: false | |
NPM_REGISTRY_TOKEN: | |
description: Authentication for the private npm registry. | |
required: false | |
GITHUB_USER_TOKEN: | |
description: Authentication token with write permission needed by the release bot (falls back to GITHUB_TOKEN). | |
required: false |
I also updated the description to align it with other workflows.
@@ -69,4 +72,5 @@ jobs: | |||
- name: Release | |||
env: | |||
GITHUB_TOKEN: ${{ secrets.GITHUB_USER_TOKEN != '' && secrets.GITHUB_USER_TOKEN || secrets.GITHUB_TOKEN }} | |||
NPM_TOKEN: ${{ secrets.NPM_REGISTRY_TOKEN != '' && secrets.NPM_REGISTRY_TOKEN || secrets.GITHUB_TOKEN }} |
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.
Let's align this with other workflows. Please
- move this to the job-wide
env
declaration aboveHAS_CONFIG
(line 18) - rename the env variable to
NODE_AUTH_TOKEN
(this is automatically read byactions/setup-node
; see documentation)
Also, is falling back to GITHUB_TOKEN
a real scenario? We don't do this in other workflows, so I wonder whether this is needed. An example that doesn't do this:
NODE_AUTH_TOKEN: ${{ secrets.NPM_REGISTRY_TOKEN }} |
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 add the following input:
NPM_REGISTRY_DOMAIN:
description: Domain of the private npm registry.
default: https://npm.pkg.github.com/
required: false
type: string
Then, use this in the "Set up node" step like this:
- name: Set up node
uses: actions/setup-node@v4
with:
node-version-file: semantic-release-repo/package.json
registry-url: ${{ inputs.NPM_REGISTRY_DOMAIN }}
Note: I renamed the step name from "Setup Node.js" to align with other workflows.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Exposes a config to be able to define the NPM_TOKEN used by the release workflow to publish to NPM registry
What is the current behavior? (You can also link to an open issue here)
There is no way to set this value
What is the new behavior (if this is a feature change)?
A new secret config is available
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No
Other information: