Skip to content
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

Create npm-publish.yml #636

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GONNAGITU72
Copy link

@GONNAGITU72 GONNAGITU72 commented Feb 1, 2024

Single

Purpose / Goal

Type

Please mention the type of PR

  • Bug Fix
  • Refactoring / Technology upgrade
  • New Feature

Note : Please ensure that you've read contribution guidelines before raising this PR. If your PR is in progress, please prepend [WIP] in PR title. Your PR will be reviewed when [WIP] will be removed from the PR title.

Bookmark this repository for further updates.

@amitguptagwl
Copy link
Member

Thanks for raising a pR. I need to understand the advantage of this addition. Currently, I prefer to check the PR thoroughly on my local and then publish it with 2FA.

Copy link
Contributor

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

@amitguptagwl IHMO using such a workflow offers certain advantages over a manual process, whatever the workflow trigger.
Here, the workflow starts when a GitHub release is created, but it could start when a tag is pushed or by letting a maintainer trigger it manually (with workflow_dispatch).

Automation ensures that the source code of a version of the npm package can be easily audited. The git tag linked to the version is always available in the repository, avoiding problems like #716.

The content of the npm package can be trusted using npm provenance.
This allows you to publicly and verifiably establish where and how your package was built, which increases supply chain security for people who consume your package.

Comment on lines +14 to +15
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Use v4 for both actions (they were probably not released when the PR has been created 😄 )
This also applies to the other job in this workflow file.

registry-url: https://registry.npmjs.org/
- run: npm ci
- run: npm publish
env:single
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: extra "single" expression

Suggested change
env:single
env:

publish-npm:
needs: build
runs-on: ubuntu-latest
steps:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Add permissions to the GH_TOKEN to be able to use npm provenance
See https://docs.npmjs.com/generating-provenance-statements

Suggested change
steps:
permissions:
contents: read
id-token: write // Give permission to mint an ID-token for npm provenance
steps:

node-version: 16
registry-url: https://registry.npmjs.org/
- run: npm ci
- run: npm publish
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- run: npm publish
- run: npm publish --provenance

@amitguptagwl
Copy link
Member

Thanks @tbouffard . let me check this in detail. I'm holding any change in FXP for some period now until some bug is reported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants