Skip to content

Comments

chore(bindings/nodejs): upgrade deps napi-rs to 3.0#6482

Merged
Xuanwo merged 4 commits intoapache:mainfrom
kingsword09:upgrade-napi-3
Aug 4, 2025
Merged

chore(bindings/nodejs): upgrade deps napi-rs to 3.0#6482
Xuanwo merged 4 commits intoapache:mainfrom
kingsword09:upgrade-napi-3

Conversation

@kingsword09
Copy link
Contributor

Which issue does this PR close?

Closes #6475.

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. releases-note/chore The PR has a title that begins with "chore" or changes other small things that hard to tell labels Aug 4, 2025
@kingsword09 kingsword09 marked this pull request as draft August 4, 2025 08:14
- uses: pnpm/action-setup@v4
with:
version: 8
version: 10.14.0
Copy link
Member

Choose a reason for hiding this comment

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

Change this can introduce a big changes to pnpm.lock, how about we bump this first?

Copy link
Contributor Author

@kingsword09 kingsword09 Aug 4, 2025

Choose a reason for hiding this comment

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

I noticed the official napi template uses pnpm 10, so I upgraded here. Now I’ll revert this change as suggested.

@kingsword09 kingsword09 marked this pull request as ready for review August 4, 2025 09:31
@kingsword09 kingsword09 requested a review from Xuanwo August 4, 2025 09:31
with:
toolchain: stable
targets: ${{ matrix.settings.target }}
- name: Cache cargo
Copy link
Member

Choose a reason for hiding this comment

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

I suggest we don't cache here. Our release workflow only run twice a month.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

.cargo-cache
target/
key: ${{ matrix.settings.target }}-cargo-${{ matrix.settings.host }}
- uses: goto-bus-stop/setup-zig@v2
Copy link
Member

Choose a reason for hiding this comment

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

Please check can we use this action first. And I remember that we have a zig related action, are we using the same one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

with:
toolchain: stable
targets: ${{ matrix.settings.target }}
- name: Cache cargo
Copy link
Member

Choose a reason for hiding this comment

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

The same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

cache: pnpm
cache-dependency-path: "bindings/nodejs/pnpm-lock.yaml"
- name: Corepack
run: npm i -g --force corepack@latest && corepack enable
Copy link
Member

Choose a reason for hiding this comment

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

So we don't need this anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the official documentation URL is as follows: docs

@kingsword09 kingsword09 requested a review from Xuanwo August 4, 2025 10:00
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thank you for working on this!

@Xuanwo Xuanwo merged commit 2a0f639 into apache:main Aug 4, 2025
294 checks passed
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 4, 2025
@kingsword09 kingsword09 deleted the upgrade-napi-3 branch August 4, 2025 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer releases-note/chore The PR has a title that begins with "chore" or changes other small things that hard to tell size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants