-
Notifications
You must be signed in to change notification settings - Fork 253
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
chore(*): Publish packages with npm provenance #1891
chore(*): Publish packages with npm provenance #1891
Conversation
🦋 Changeset detectedLatest commit: 1a66fb1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@@ -17,6 +17,6 @@ module.exports = { | |||
'subject-case': [2, 'always', ['sentence-case']], | |||
'body-max-line-length': [1, 'always', '150'], | |||
'scope-empty': [2, 'never'], | |||
'scope-enum': [2, 'always', [...getPackageNames(), 'repo', 'release']], | |||
'scope-enum': [2, 'always', [...getPackageNames(), 'repo', 'release', '*']], |
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.
If you want to write a commit message that affects everything inside packages
, we should use *
. So this just adds it as it was missing
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.
This was missing, indeed! Great job!
@@ -15,6 +15,9 @@ jobs: | |||
name: Release | |||
if: ${{ github.repository == 'clerkinc/javascript' }} | |||
runs-on: ${{ vars.RUNNER_LARGE }} | |||
permissions: |
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.
!snapshot |
This comment was marked as outdated.
This comment was marked as outdated.
!snapshot |
This comment was marked as outdated.
This comment was marked as outdated.
This reverts commit 9ff9834.
.github/workflows/release-prod.yml
Outdated
@@ -15,6 +15,11 @@ jobs: | |||
name: Release | |||
if: ${{ github.repository == 'clerkinc/javascript' }} | |||
runs-on: ${{ vars.RUNNER_LARGE }} | |||
env: | |||
NPM_CONFIG_PROVENANCE: true |
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.
💡 We might be able to add provenance=true
to the root .npmrc
file in the repo, instead of needing to add the environment variable to each release workflow.
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.
I tried that in b6c514a
(#1891) but Verdaccio didn't like that: https://github.com/clerkinc/javascript/actions/runs/6533197979/job/17737954907?pr=1891
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.
@LekoArts It looks like that error might have been because the id-token
permission wasn't set yet 👀
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.
I tried that in 3545ced
(#1891), that didn't make a difference
!snapshot |
This comment was marked as outdated.
This comment was marked as outdated.
"nuke": "./scripts/nuke.sh", | ||
"yalc:all": "for d in packages/*/; do echo $d; cd $d; yalc push --replace --sig; cd '../../'; done", | ||
"prepare": "husky install", | ||
"changeset": "npx changeset", | ||
"changeset": "changeset", |
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.
There's no need to use npx
, in fact I believe the current setup didn't allow for environment variables (https://brianchildress.co/environment-variables-using-npx/)
…sing-the-provenance-flag
…sing-the-provenance-flag
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.
Good job with this @LekoArts
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.
Great job here @LekoArts 💯
This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
This PR enables our packages to be published with npm provenance:
https://docs.npmjs.com/generating-provenance-statements
Since we are using changesets (ref: changesets/changesets#1152) we can other ways of doing that: https://docs.npmjs.com/generating-provenance-statements#using-third-party-package-publishing-tools
I also added the
directory
torepository
(https://docs.npmjs.com/cli/v10/configuring-npm/package-json#repository) because provenance needs this: https://docs.npmjs.com/generating-provenance-statements#prerequisitesChecklist
npm test
runs as expected.npm run build
runs as expected.Type of change
Packages affected
@clerk/clerk-js
@clerk/clerk-react
@clerk/nextjs
@clerk/remix
@clerk/types
@clerk/themes
@clerk/localizations
@clerk/clerk-expo
@clerk/backend
@clerk/clerk-sdk-node
@clerk/shared
@clerk/fastify
@clerk/chrome-extension
gatsby-plugin-clerk
build/tooling/chore