Skip to content

Comments

refactor(bindings/node)!: Change is_exist to exists to align with nodejs API style#5731

Merged
Xuanwo merged 6 commits intoapache:mainfrom
yihong0618:hy/chore_node_binding
Mar 11, 2025
Merged

refactor(bindings/node)!: Change is_exist to exists to align with nodejs API style#5731
Xuanwo merged 6 commits intoapache:mainfrom
yihong0618:hy/chore_node_binding

Conversation

@yihong0618
Copy link
Contributor

@yihong0618 yihong0618 commented Mar 10, 2025

Which issue does this PR close?

this patch chore node biding also from commnet #5717

and also chore the package.json version to fix audit.

@yihong0618 yihong0618 requested a review from suyanhanx as a code owner March 10, 2025 14:37
@github-actions github-actions bot added the releases-note/chore The PR has a title that begins with "chore" or changes other small things that hard to tell label Mar 10, 2025
Xuanwo
Xuanwo reviewed Mar 10, 2025
View reviewed changes
@Xuanwo
Copy link
Member

Xuanwo commented Mar 11, 2025

Nodejs does have an exists API but deprecated: https://nodejs.org/api/fs.html#fsexistspath-callback

Maybe we can add access API in the future.

@Xuanwo Xuanwo changed the title chore: node binding is_exist to exists and fix audit chore: node binding is_exist to exists Mar 11, 2025
@Xuanwo
Copy link
Member

Xuanwo commented Mar 11, 2025

Hi @yihong0618, this update will introduce a breaking change to Node.js.

Would you be willing to update upgrade.md so we can inform users about it? It's also fine to start another PR for this.

@Xuanwo Xuanwo changed the title chore: node binding is_exist to exists refactor!(bindings/node): Change is_exist to exists to align with nodejs API style Mar 11, 2025
@Xuanwo Xuanwo changed the title refactor!(bindings/node): Change is_exist to exists to align with nodejs API style refactor(bindings/node)!: Change is_exist to exists to align with nodejs API style Mar 11, 2025
@github-actions github-actions bot added the releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" label Mar 11, 2025
@yihong0618
Copy link
Contributor Author

yihong0618 commented Mar 11, 2025

will do this in this PR

@Xuanwo
Copy link
Member

Xuanwo commented Mar 11, 2025

Also, this change is not a chore, it's big refactor that changed nodejs users API. I have updated the PRs title.

@Xuanwo Xuanwo removed the releases-note/chore The PR has a title that begins with "chore" or changes other small things that hard to tell label Mar 11, 2025
@yihong0618
Copy link
Contributor Author

yihong0618 commented Mar 11, 2025

Also, this change is not a chore, it's big refactor that changed nodejs users API. I have updated the PRs title.

learned that thanks

Xuanwo
Xuanwo reviewed Mar 11, 2025
View reviewed changes
bindings/nodejs/upgrade.md Outdated
@@ -1,3 +1,11 @@
# Upgrade to v0.53
Copy link
Member

@Xuanwo Xuanwo Mar 11, 2025

Choose a reason for hiding this comment

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

The next version of nodejs will be 0.48:

opendal/bindings/nodejs/package.json

Line 4 in 3cfce47

"version": "0.47.11",

Xuanwo
Xuanwo approved these changes Mar 11, 2025
View reviewed changes
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 @yihong0618 for this change!

@Xuanwo Xuanwo merged commit 5d63451 into apache:main Mar 11, 2025
182 of 183 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

@suyanhanx suyanhanx Awaiting requested review from suyanhanx suyanhanx is a code owner

Assignees

No one assigned

Labels

releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor"

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@yihong0618 @Xuanwo