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

added example usage of set datastructure #2534

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

avinashkrishna613
Copy link
Contributor

Description

I faced some difficulties while working on sets and later found the value for set should be string. So i just add the example for how to use set datastructure in a node application.

@leibale
Copy link
Contributor

leibale commented Jun 27, 2023

Maybe just extend and rename this example instead of creating a new one?

BTW, respect for facing some difficulties, solving them yourself, and then trying to help others as well! :)

@avinashkrishna613
Copy link
Contributor Author

Made changes as suggested @leibale

@leibale
Copy link
Contributor

leibale commented Jun 29, 2023

@avinashkrishna613 I made some more changes, I think it's ready.. wanna review my changes please? 🙏

examples/set.js Outdated
console.log(member);
});
iCursor = cursor;
} while (iCursor !== 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can keep this. Let's say for example, if user needs some kind of pagination or something then he can make use of this method.
Rest everything looks fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

@avinashkrishna613 this is exactly how "sScanIterator" works, there is no reason to implement it again..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@leibale Then it's fine.

@leibale leibale requested review from simonprickett and guyroyse July 1, 2023 06:54
Copy link
Contributor

@simonprickett simonprickett left a comment

Choose a reason for hiding this comment

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

This also needs to update the README.md file to remove the old set example and add the new one: /~https://github.com/redis/node-redis/blob/master/examples/README.md

@avinashkrishna613
Copy link
Contributor Author

Updated readme @simonprickett

Copy link
Contributor

@simonprickett simonprickett left a comment

Choose a reason for hiding this comment

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

Approved with suggestion.

examples/README.md Outdated Show resolved Hide resolved
@avinashkrishna613
Copy link
Contributor Author

Any update on this @guyroyse ?

@avinashkrishna613
Copy link
Contributor Author

Anything needs to be done on this? @leibale @simonprickett

Co-authored-by: Simon Prickett <simon@crudworks.org>
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.

3 participants