Skip to content

Preparing for NPM trusted publishing plus node version housekeeping #2952

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.

Sign up for GitHub

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
+3 −2
Draft

Preparing for NPM trusted publishing plus node version housekeeping #2952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b037053
Added id-token permission to allow for migration to npm trusted
smallsaucepan Nov 16, 2025
25fc296
Renamed turf.yml more descriptively to ci.yml
smallsaucepan Nov 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/turf.yml → .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:

strategy:
matrix:
node-version: [18.x, 20.x, 22.x]
node-version: [20.x, 22.x, 24.x]
Copy link
Collaborator

@mfedderly mfedderly Nov 19, 2025

Choose a reason for hiding this comment

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

I've seen other packages consider dropping support for old versions of nodejs as a breaking change.
We aren't technically breaking support here, just not testing it which would make it more likely for a breaking change to sneak in.

I'm happy to either merge this as a non-major change, or push it off for later if you'd prefer.

Copy link
Member Author

@smallsaucepan smallsaucepan Nov 20, 2025

Choose a reason for hiding this comment

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

Happy to tread cautiously. So add 18 back in and keep 24 too? Or cap it at 3 - 18, 20, 22?

Copy link
Collaborator

@mfedderly mfedderly Nov 20, 2025

Choose a reason for hiding this comment

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

Yeah I think we can just add 24. Once this merges we need to make 24 required.


steps:
- name: Checkout
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ on:
- "v*.*.*"

permissions:
id-token: write
Copy link
Collaborator

@mfedderly mfedderly Nov 19, 2025

Choose a reason for hiding this comment

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

Were you able to configure a trusted publisher on npmjs? Otherwise I can dig into doing that.

Copy link
Member Author

@smallsaucepan smallsaucepan Nov 20, 2025

Choose a reason for hiding this comment

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

I haven't set up that side of it yet, so be my guest!

Copy link
Collaborator

@mfedderly mfedderly Nov 20, 2025

Choose a reason for hiding this comment

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

Oof, I don't see a way to do it for all of the packages at once. I'll probably click through all of them one at a time later today 😩.

I'm going to configure it like this. Note that I added a permissive release environment in GitHub now, which we can configure later without having to go to all 1xx packages and reconfigure it later.

image

After that, per the docs guidance, I will then also set this:

image

contents: write

jobs:
Expand All @@ -14,7 +15,7 @@ jobs:
fail-fast: true
matrix:
node:
- 18
- 20
platform:
- ubuntu-latest
name: "${{matrix.platform}} / Node.js ${{ matrix.node }}"
Expand Down