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

#2285-Add-example-scripts-hyperloglog - Added hyperloglog examples to… #2289

Merged

Conversation

ade1705
Copy link
Contributor

@ade1705 ade1705 commented Oct 10, 2022

Added example scripts for the hyperloglog as requested - #2285

created a hyperloglog.js file

Checklist

  • Does npm test pass with this change (including linting)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

@SuzeShardlow
Copy link
Contributor

Closes #2285 when merged.

@@ -0,0 +1,51 @@
//Example to log traffic data at intersections for the city of San Francisco.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put a space after // in comments throughout, thanks.

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.

Thank you - looking good, the code works well thanks. I've left a couple of suggestions, please ensure all comments have a space between the // and the text, and that sentences end with a full stop / period... so:

// this is a comment.

rather than:

//this is a comment

Thank you!

@@ -0,0 +1,51 @@
//Example to log traffic data at intersections for the city of San Francisco.
//Log license plates of each car scanned at each intersection and add to the intersections Hyperloglog
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//Log license plates of each car scanned at each intersection and add to the intersections Hyperloglog
// Log license plates of each car scanned at each intersection and add to the intersections Hyperloglog.


await client.connect();

//To Create a HyperLogLog, the `pfAdd` method is used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//To Create a HyperLogLog, the `pfAdd` method is used.
// Use `pfAdd` to add an element to a Hyperloglog, creating the Hyperloglog if necessary.

@ade1705
Copy link
Contributor Author

ade1705 commented Oct 11, 2022

@simonprickett Thanks for the feedback, Changes made

@ade1705 ade1705 requested review from simonprickett and removed request for SuzeShardlow October 11, 2022 14:39
@SuzeShardlow
Copy link
Contributor

Thanks @ade1705, we'll review this! In the meantime, if you're a member of our Discord server (https://discord.gg/redis), please let us know your handle. Thanks!

@ade1705
Copy link
Contributor Author

ade1705 commented Oct 12, 2022

Thanks @ade1705, we'll review this! In the meantime, if you're a member of our Discord server (https://discord.gg/redis), please let us know your handle. Thanks!

Yea, I am a member of the discord, my handle @TrussDamola

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.

Looks good to me and works when tested. Thank you!

@SuzeShardlow
Copy link
Contributor

Closes #2285. Thanks @ade1705! I'll give you the Hacktoberfest 2022 role on Discord :)

@SuzeShardlow SuzeShardlow merged commit 9398e5d into redis:master Oct 14, 2022
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.

None yet

3 participants