Skip to content

fix(publish.yml): globally install node. use correct github action name#1012

Merged
DreaminDani merged 2 commits intomainfrom
dani-fix-node-version
Apr 21, 2026
Merged

fix(publish.yml): globally install node. use correct github action name#1012
DreaminDani merged 2 commits intomainfrom
dani-fix-node-version

Conversation

@DreaminDani
Copy link
Copy Markdown
Contributor

iteration is key

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 21, 2026

⚠️ No Changeset found

Latest commit: eea3ad4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 21, 2026

Claude finished @DreaminDani's task in 42s —— View job


PR Review: fix(publish.yml): globally install node. use correct github action name

  • Read changed files and gather context
  • Review the workflow changes
  • Post final review

✅ Looks great! No issues found — nice work!

Two clean, correct fixes in .github/workflows/publish.yml:

  1. Git bot identity (lines 62–63): Switching from "GitHub Actions" <actions@clickhouse.com> to "github-actions[bot]" <github-actions[bot]@users.noreply.github.com> is the proper canonical identity for GitHub Actions bot commits. This ensures automated commits are attributed correctly in GitHub's UI and git history.

  2. Comment update (lines 82–83): Replacing the stale inline note (# Node 22 includes npm >= 11.5.1) with an explanation of why npm install -g npm@latest should be avoided on hosted runners is a helpful breadcrumb for future maintainers — especially given this PR is itself the result of learning that lesson. 👍

No component changes, no breaking changes, no concerns.

Comment thread .github/workflows/publish.yml
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 21, 2026

Claude finished @DreaminDani's task in 1m 5s —— View job


  • Read changed files and gather context
  • Review the workflow changes
  • Post final review

PR Review: fix(publish.yml): globally install node. use correct github action name

The git identity fix is clean and correct. One issue worth resolving before merge:

🚨 Must Fix

  • .github/workflows/publish.yml lines 82–88: The comment directly contradicts the code. Line 82–83 says "Avoid npm install -g npm@latest on hosted runners — it can leave npm broken (e.g. MODULE_NOT_FOUND / promise-retry)", but lines 85–88 then run exactly that command. This will confuse future maintainers.

    If the intent is to keep the upgrade step (which the commit message "add npm upgrade" suggests), the comment should explain why it's safe here or what precondition makes it work (e.g. "must run after setup-node with registry-url set to avoid the MODULE_NOT_FOUND issue"). If the upgrade step is still broken, it should be removed and the comment stands as a guard rail.

    Fix this →

✅ Looks good

  • Git bot identity (github-actions[bot] / github-actions[bot]@users.noreply.github.com) is the correct canonical form — nice fix.
  • registry-url is properly set on setup-node, which is required for OIDC provenance publishing.

@DreaminDani DreaminDani merged commit 0e4c652 into main Apr 21, 2026
7 checks passed
@DreaminDani DreaminDani deleted the dani-fix-node-version branch April 21, 2026 21:15
@workflow-authentication-public
Copy link
Copy Markdown
Contributor

📚 Storybook Preview Deployed

✅ Preview URL: https://click-84z896gny-clickhouse.vercel.app

Built from commit: 570a9fdbc147330e9c28bb31226ddb2bdecfb64d

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.

2 participants