Update npm-release.yml #853
Conversation
WalkthroughThis pull request updates the npm release workflow by introducing a new boolean input parameter Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/Trigger
participant R as Release Job
participant N as Node Step (Build)
participant NR as Node-Release Step
participant P as Publish Step
U->>R: Dispatch with skip-deployment input
R->>N: Execute build step
N-->>R: Return artifact name
R->>NR: Execute version retrieval step
NR-->>R: Return version info
alt skip-deployment is false
R->>P: Execute npm publish command
else
R->>R: Skip deployment step
end
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/npm-release.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/npm-release.yml
[error] 22-22: wrong indentation: expected 8 but found 7
(indentation)
[error] 24-24: trailing spaces
(trailing-spaces)
[warning] 40-40: too few spaces before comment
(comments)
[warning] 53-53: too few spaces before comment
(comments)
🪛 actionlint (1.7.4)
.github/workflows/npm-release.yml
37-37: property "node-release" is not defined in object type {node: {conclusion: string; outcome: string; outputs: {string => string}}; npm_release: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
53-53: property "npm-token" is not defined in object type {app-path: string; release-version: string; skip-deployment: bool}
(expression)
| ARTIFACT_NAME: ${{ steps.npm_release.outputs.ARTIFACT_NAME }} | ||
| ARTIFACT_VERSION: ${{steps.npm_release.outputs.ARTIFACT_VERSION}} | ||
| ARTIFACT_NAME: ${{ steps.node.outputs.artifact-name }} | ||
| ARTIFACT_VERSION: ${{steps.node-release.outputs.VERSION}} | ||
| steps: | ||
| - id: npm_release | ||
| uses: it-at-m/lhm_actions/action-templates/actions/action-npm-release@0adbb607339cb32296289c8c319970b9ef2121cb # v1.0.2 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve Inline Comment Spacing.
For enhanced readability, consider adding an extra space before inline comments. For example, adjust the comment on the action reference on line 40.
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 40-40: too few spaces before comment
(comments)
.github/workflows/npm-release.yml
Outdated
| ARTIFACT_NAME: ${{ steps.node.outputs.artifact-name }} | ||
| ARTIFACT_VERSION: ${{steps.node-release.outputs.VERSION}} |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Verify Output Reference for Artifact Version.
The output variable ARTIFACT_VERSION is set using ${{ steps.node-release.outputs.VERSION }}, yet no step with the ID node-release exists in this job. Please verify whether the step ID should be updated (e.g., to node) or if a new step is intended to be added.
Action Required: Correct the Artifact Version Step Reference
The output for ARTIFACT_VERSION incorrectly points to ${{ steps.node-release.outputs.VERSION }}: no step with the ID node-release exists in this workflow. It appears the intent was to use the already defined node step (as done for ARTIFACT_NAME). Please update the reference to use steps.node.outputs.VERSION or add a missing step if that was the intent.
- File:
.github/workflows/npm-release.yml - Lines: 36-37
- Suggested Diff:
ARTIFACT_NAME: ${{ steps.node.outputs.artifact-name }} - ARTIFACT_VERSION: ${{steps.node-release.outputs.VERSION}} + ARTIFACT_VERSION: ${{ steps.node.outputs.VERSION }}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ARTIFACT_NAME: ${{ steps.node.outputs.artifact-name }} | |
| ARTIFACT_VERSION: ${{steps.node-release.outputs.VERSION}} | |
| ARTIFACT_NAME: ${{ steps.node.outputs.artifact-name }} | |
| ARTIFACT_VERSION: ${{ steps.node.outputs.VERSION }} |
🧰 Tools
🪛 actionlint (1.7.4)
37-37: property "node-release" is not defined in object type {node: {conclusion: string; outcome: string; outputs: {string => string}}; npm_release: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
| env: | ||
| NODE_AUTH_TOKEN: ${{ inputs.npm-token }} # Centralized token in it-at-m GitHub organization |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Define the npm-token Input for Environment Variable.
The npm publish step is referencing ${{ inputs.npm-token }} to set NODE_AUTH_TOKEN, but no input parameter npm-token is defined in the workflow_dispatch inputs. This will cause a runtime error when the token is needed for publishing. In addition, consider adding proper spacing before inline comments for clarity.
Proposed fix: Add a new input parameter for npm-token in the inputs block, for example:
inputs:
release-version:
type: choice
description: "Select version increment type (follows Semantic Versioning)"
required: true
options:
- patch
- minor
- major
app-path:
type: choice
description: "Select the node service to release"
required: true
default: refarch-frontend
options:
- refarch-frontend # Add or modify frontend paths according to your project structure
+ npm-token:
+ type: string
+ description: "NPM authentication token required for publishing"
+ required: true
skip-deployment:
default: true
type: boolean
description: "skip deployment to npm registry"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env: | |
| NODE_AUTH_TOKEN: ${{ inputs.npm-token }} # Centralized token in it-at-m GitHub organization | |
| inputs: | |
| release-version: | |
| type: choice | |
| description: "Select version increment type (follows Semantic Versioning)" | |
| required: true | |
| options: | |
| - patch | |
| - minor | |
| - major | |
| app-path: | |
| type: choice | |
| description: "Select the node service to release" | |
| required: true | |
| default: refarch-frontend | |
| options: | |
| - refarch-frontend # Add or modify frontend paths according to your project structure | |
| npm-token: | |
| type: string | |
| description: "NPM authentication token required for publishing" | |
| required: true | |
| skip-deployment: | |
| default: true | |
| type: boolean | |
| description: "skip deployment to npm registry" |
🧰 Tools
🪛 actionlint (1.7.4)
53-53: property "npm-token" is not defined in object type {app-path: string; release-version: string; skip-deployment: bool}
(expression)
🪛 YAMLlint (1.35.1)
[warning] 53-53: too few spaces before comment
(comments)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/npm-release.yml (1)
1-20: 🛠️ Refactor suggestion
⚠️ Potential issueAdd Missing Input Parameter for
npm-token
The workflow’s npm publish step (line 52) relies on${{ inputs.npm-token }}, but there is no correspondingnpm-tokeninput defined. This will lead to a runtime error when trying to publish the package. Please add thenpm-tokeninput to theworkflow_dispatch.inputsblock.options: - refarch-frontend # Add or modify frontend paths according to your project structure + npm-token: + type: string + description: "NPM authentication token required for publishing" + required: true(Place the new input ideally after
app-pathand beforeskip-deployment.)
♻️ Duplicate comments (2)
.github/workflows/npm-release.yml (2)
52-52: 🧹 Nitpick (assertive)Improve Inline Comment Spacing
For better readability, consider adding an extra space before the inline comment on theNODE_AUTH_TOKENenvironment variable.- NODE_AUTH_TOKEN: ${{ inputs.npm-token }} # Centralized token in it-at-m GitHub organization + NODE_AUTH_TOKEN: ${{ inputs.npm-token }} # Centralized token in it-at-m GitHub organization🧰 Tools
🪛 actionlint (1.7.4)
52-52: property "npm-token" is not defined in object type {app-path: string; release-version: string; skip-deployment: bool}
(expression)
35-36: 🛠️ Refactor suggestion
⚠️ Potential issueFix Output Reference for ARTIFACT_VERSION
The output variableARTIFACT_VERSIONstill referencessteps.node-release.outputs.VERSION, but no step with the IDnode-releaseexists. Update the reference to use the definednodestep instead.- ARTIFACT_VERSION: ${{steps.node-release.outputs.VERSION}} + ARTIFACT_VERSION: ${{ steps.node.outputs.VERSION }}🧰 Tools
🪛 actionlint (1.7.4)
36-36: property "node-release" is not defined in object type {node: {conclusion: string; outcome: string; outputs: {string => string}}; npm_release: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/npm-release.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/npm-release.yml
36-36: property "node-release" is not defined in object type {node: {conclusion: string; outcome: string; outputs: {string => string}}; npm_release: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
52-52: property "npm-token" is not defined in object type {app-path: string; release-version: string; skip-deployment: bool}
(expression)
| skip-deployment: | ||
| default: true | ||
| type: boolean | ||
| description: "skip deployment to npm registry" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure Correct YAML Formatting for skip-deployment Input
The skip-deployment input is now introduced with the correct boolean type and default value. Please double-check that the indentation is consistent with other inputs and that there is no trailing whitespace (especially on the description line).
Reference
Issue: it-at-m/.github#109
Summary by CodeRabbit