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.

Sign up for GitHub

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

Jump to bottom

Update Redis README #1357

Merged
merged 2 commits into from
Jun 11, 2019
Merged

Update Redis README #1357

merged 2 commits into from
Jun 11, 2019

Conversation

grant
Copy link
Contributor

@grant grant commented Jun 11, 2019 •
edited
Loading

Related to #1032, b/133157483

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 11, 2019
@grant grant requested a review from ace-n June 11, 2019 01:55
ace-n
ace-n reviewed Jun 11, 2019
View reviewed changes
memorystore/redis/README.md Outdated
node server.js
```

Visit the following `localhost` URL to test your server that connects to Redis and observe a persistant "Vistor number".
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: vistor -> visitor (add i)

memorystore/redis/README.md

To run this sample locally, follow these instructions:

In one terminal, [run Redis](https://redis.io/topics/quickstart):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it better to refer to Redis Labs' hosted redis solution here? (Maybe a question for the relevant PM.)

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 the official Redis quickstart is better than that site for learning about what Redis is.

@grant
Copy link
Contributor Author

grant commented Jun 11, 2019 •
edited
Loading

@ace-n Please add more comments or approve :)
Are you waiting for Gopal's response? If so, I don't think this change needs that PM's approval for the lack of instructions on how to run this sample.

ace-n
ace-n approved these changes Jun 11, 2019
View reviewed changes
Copy link
Contributor

@ace-n ace-n left a comment

Choose a reason for hiding this comment

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

On second thought - looks like the linked tutorial assumes you're running your own Redis instance (and not using Redis Labs or some other hosted service).

Considering that, this should be good to go.

@grant grant merged commit 8bed482 into master Jun 11, 2019
@grant grant mentioned this pull request Jun 11, 2019
Closed
@grant grant deleted the grant-redis-readme-localhost branch July 19, 2019 20:19
@dlminvestments dlminvestments mentioned this pull request Jan 10, 2023
Merged
This was referenced Jun 20, 2023
Open
Open
Open
Open
Open
@aqualen aqualen mentioned this pull request Nov 28, 2023
Open
@aqualen aqualen mentioned this pull request Dec 14, 2023
Open
@Matthelonianxl Matthelonianxl mentioned this pull request Dec 15, 2023
Open
@aqualen aqualen mentioned this pull request Jan 5, 2024
Open
@Matthelonianxl Matthelonianxl mentioned this pull request Jan 5, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ace-n ace-n ace-n approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@grant @ace-n @googlebot