Skip to content
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.

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

Node: Fixed missing exports #2301

Merged

Conversation

yipin-chen
Copy link
Collaborator

@yipin-chen yipin-chen commented Sep 16, 2024

Fixed missing exports:

  • Script
  • ObjectType
  • ClusterScanCursor
  • BaseClientConfiguration
  • GlideClusterClientConfiguration
  • LevelOptions

This PR will resolve issue #2307

@yipin-chen yipin-chen marked this pull request as ready for review September 16, 2024 23:43
@yipin-chen yipin-chen requested a review from a team as a code owner September 16, 2024 23:43
@Yury-Fridlyand
Copy link
Collaborator

Add export for ObjectType please, too. https://github.com/Bit-Quill/valkey-glide/actions/runs/10893620266/job/30230895039#step:5:7251

Copy link
Collaborator

@acarbonetto acarbonetto left a comment

Choose a reason for hiding this comment

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

Please also fix the examples application for node:
https://github.com/valkey-io/valkey-glide/blob/main/examples/node/index.ts#L52
to:

    const pong = await client.customCommand(["PING"], { route: "randomNode" });

@yipin-chen yipin-chen changed the title Add support to export Script Add support to export Script and ObjectType Sep 17, 2024
Copy link
Collaborator

@GumpacG GumpacG left a comment

Choose a reason for hiding this comment

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

Please add CHANGELOG entry

@yipin-chen yipin-chen linked an issue Sep 17, 2024 that may be closed by this pull request
Copy link
Collaborator

@acarbonetto acarbonetto left a comment

Choose a reason for hiding this comment

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

Need to verify that the changes here are complete by running IT against the release candidate.

@Yury-Fridlyand
Copy link
Collaborator

Yury-Fridlyand commented Sep 18, 2024

I just found that ClusterScanCursor, BaseClientConfiguration and GlideClusterClientConfiguration are also not exported 🤦
UPD Need to export LevelOptions too

@acarbonetto
Copy link
Collaborator

according to https://github.com/Bit-Quill/valkey-glide/actions/runs/10929789563/job/30341270532
Only failure spotted when running IT tests against the RC shows missing Script and ObjectType exports.
cc: @prateek-kumar-improving

CHANGELOG.md Outdated Show resolved Hide resolved
@yipin-chen yipin-chen changed the title Add support to export Script and ObjectType Fixed missing exports Sep 19, 2024
@yipin-chen yipin-chen changed the title Fixed missing exports Node: Fixed missing exports Sep 19, 2024
Signed-off-by: Yi-Pin Chen <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
@acarbonetto acarbonetto merged commit e37308d into valkey-io:release-1.1 Sep 19, 2024
9 checks passed
@acarbonetto acarbonetto deleted the node/yipin-fix-Script-export branch September 19, 2024 06:27
shohamazon pushed a commit to shohamazon/glide-for-redis that referenced this pull request Sep 23, 2024
Node: include missing exports in the npm release

Signed-off-by: Yi-Pin Chen <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node: failed to export Script and ObjectType
6 participants