Skip to content

docs(readme): create options, add more headings #132

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

Merged
merged 4 commits into from
Feb 25, 2022

Conversation

julie-ng
Copy link
Contributor

As a developer, I want complete samples incl. minimum client configuration options, which would include the redis client password.

I had to manually open the ioredis documentation to check if it expected a password or secret configuration option. Probably the former. But including an example here saves first time users the trouble of looking it up - thanks 🙂 🙏

Original

In the original, only the hostname was provided.

fastify.register(require('fastify-redis'), { host: '127.0.0.1' })
// or
fastify.register(require('fastify-redis'), { url: 'redis://127.0.0.1', /* other redis options */ })

Btw - why was it host in the first example and then url?

Suggestion

Changes the 2nd example to include some common options, esp. password

fastify.register(require('fastify-redis'), { 
  host: '127.0.0.1', 
  password: '***',
  port: 6379, // Redis port
  family: 4   // 4 (IPv4) or 6 (IPv6)
})

Checklist

Copy link
Member

@darkgl0w darkgl0w 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 your PR :)

A little nit.
LGTM with the suggested change. ^^

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@darkgl0w darkgl0w left a comment

Choose a reason for hiding this comment

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

LGTM with @Fdawgs suggested changes. ^^

Co-authored-by: Frazer Smith <frazer.dev@outlook.com>
@julie-ng
Copy link
Contributor Author

@Fdawgs okay your changes have been applied :)

Co-authored-by: darkgl0w <31093081+darkgl0w@users.noreply.github.com>
@darkgl0w darkgl0w merged commit 727a27e into fastify:master Feb 25, 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.

5 participants