Skip to content

chore: update to OIDC publishing#1709

Merged
aklenik merged 1 commit into
mainfrom
update-pub
Sep 24, 2025
Merged

chore: update to OIDC publishing#1709
aklenik merged 1 commit into
mainfrom
update-pub

Conversation

@ryjones

@ryjones ryjones commented Sep 24, 2025

Copy link
Copy Markdown
Contributor

Checklist

This is to update from token publishing to OIDC publishing.

Issue/User story

Steps to Reproduce

Existing issues

Design of the fix

Validation of the fix

Automated Tests

What documentation has been provided for this pull request

@ryjones
ryjones requested review from a team, aklenik and davidkel September 24, 2025 14:25

@aklenik aklenik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ryjones left a minor suggestion, just to be explicit with the used versions

uses: actions/setup-node@v4
with:
node-version: 18.x
- name: Publish Caliper

@aklenik aklenik Sep 24, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd add these explicit steps (taken from the official docs at https://docs.npmjs.com/trusted-publishers#github-actions-configuration) before the "Publish Caliper" step, so we're sure that we have the required npm version (I couldn't find what npm version comes with 18.x when using setup-node):

# Ensure npm 11.5.1 or later is installed
- name: Update npm
  run: npm install -g npm@latest
- run: npm ci
- run: npm run build --if-present
- run: npm test
- run: npm publish
# rest is unchanged
- name: Publish Caliper

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

does this look good now?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ryjones I think the new block should go between L27-28 (according to the current commit, L15-16 in the previous version), just after the setup-node step

@ryjones
ryjones force-pushed the update-pub branch 2 times, most recently from 277380e to fd8fbed Compare September 24, 2025 17:18
@ryjones
ryjones requested a review from aklenik September 24, 2025 17:20

@aklenik aklenik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ryjones The structure seems okay now, but I also copied some unnecessary steps in the example, when we only need the npm update step.

The whole steps section should look like this (don't want to mess with your commit directly as a reviewer, I don't know how that would affect the PR workflow):

steps:
    - uses: actions/checkout@v4
    - name: Use Node.js 18.x
      uses: actions/setup-node@v4
      with:
        node-version: 18.x
    # anchor to the smallest npm version supporting trusted publishing
    - name: Update npm
      run: npm install -g npm@11.5.1
    - name: Publish Caliper
      run: .build/publish-caliper.sh

Signed-off-by: Ry Jones <ry@linux.com>

@aklenik aklenik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM :)

@aklenik
aklenik merged commit 6cf0011 into main Sep 24, 2025
17 checks passed
@aklenik
aklenik deleted the update-pub branch September 24, 2025 18:05
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