Fiddle: corrections for deployment#19
Conversation
There was a problem hiding this comment.
Pull request overview
This PR consolidates deployment workflows by merging the separate release.yml workflow into deploy.yml and removing the deploy-beta.yml workflow. The changes modernize the npm publishing process by switching from Docker-based publishing with NPM_TOKEN secrets to OIDC-based authentication and direct npm commands.
Changes:
- Consolidated release and deploy workflows into a single
deploy.ymlfile - Migrated from Docker-based npm publishing to direct npm CLI commands
- Updated to OIDC authentication for npm publishing instead of NPM_TOKEN secrets
- Removed separate beta deployment workflow
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.github/workflows/release.yml |
Removed - functionality merged into deploy.yml |
.github/workflows/deploy.yml |
Updated to handle both next package deployments and tagged releases, switched to OIDC authentication and direct npm publishing |
.github/workflows/deploy-beta.yml |
Removed - beta deployment functionality consolidated |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -31,8 +34,37 @@ jobs: | |||
| - name: Calculate commit count since last tag | |||
| id: vars | |||
| run: echo ::set-output name=commit_count::$(git rev-list $(git describe --tags --abbrev=0)..HEAD --count) | |||
There was a problem hiding this comment.
The set-output command using ::set-output syntax is deprecated and will be disabled in future GitHub Actions runners. This should be updated to use the modern syntax by writing to the $GITHUB_OUTPUT environment file instead.
| run: echo ::set-output name=commit_count::$(git rev-list $(git describe --tags --abbrev=0)..HEAD --count) | |
| run: | | |
| echo "commit_count=$(git rev-list $(git describe --tags --abbrev=0)..HEAD --count)" >> "$GITHUB_OUTPUT" |
| - name: Publish package to npm | ||
| if: startsWith(github.ref, 'refs/tags/v') | ||
| run: npm publish |
There was a problem hiding this comment.
The npm publish command requires authentication to publish to npm registry. While OIDC authentication permissions are set (id-token: write) and registry-url is configured in setup-node, the NODE_AUTH_TOKEN environment variable is not being set. This environment variable needs to be provided to authenticate with npm when using npm publish.
| - name: Publish next package to npm | ||
| if: steps.vars.outputs.commit_count != '0' | ||
| run: docker run -v $(pwd)/.npmrc:/root/.npmrc $DOCKER_IMAGE make NEXT_VERSION=${{ steps.vars.outputs.commit_count }} publish-next | ||
| if: steps.vars.outputs.commit_count != '0' && github.ref == 'refs/heads/main' | ||
| run: npx kiba-publish --next --next-version ${{ steps.vars.outputs.commit_count }} |
There was a problem hiding this comment.
The npx kiba-publish command likely requires npm authentication to publish packages. Similar to the standard npm publish command, this step needs the NODE_AUTH_TOKEN environment variable to be set for authentication with the npm registry.
| - name: Extract built package from Docker | ||
| run: | | ||
| docker create --name temp-container $DOCKER_IMAGE | ||
| docker cp temp-container:/app/. ./ | ||
| docker rm temp-container |
There was a problem hiding this comment.
The Docker container extraction may overwrite files in the current directory (including the .git directory and workflow files) which could cause unexpected behavior. Consider extracting to a specific subdirectory or only copying the specific built package files needed for publishing.
Description
Screenshots:
Checklist: