-
Notifications
You must be signed in to change notification settings - Fork 50
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 available installation options #223
Conversation
@madolson @zuiderkwast Please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks!
Just some minor comments.
### Description <!-- Describe what this change achieves--> ### Issues Resolved Issue valkey-io/valkey-doc#221 Rebased over #201 Related PR: valkey-io/valkey-doc#223 Changes looks: ![image](https://github.com/user-attachments/assets/694f9b8c-3dd7-4c34-a9d7-9d1dd67d4a33) ![image](https://github.com/user-attachments/assets/3aeee441-f759-42d2-812d-dc8e95515ecb) ### Check List - [x] Commits are signed per the DCO using `--signoff` By submitting this pull request, I confirm that my contribution is made under the terms of the BSD-3-Clause License. --------- Signed-off-by: Madelyn Olson <[email protected]> Signed-off-by: avifenesh <[email protected]> Co-authored-by: Madelyn Olson <[email protected]> Co-authored-by: avifenesh <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly lgtm.
Signed-off-by: avifenesh <[email protected]>
Signed-off-by: avifenesh <[email protected]>
Signed-off-by: avifenesh <[email protected]>
d0b4343
to
28fad30
Compare
@zuiderkwast @madolson Updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a comment, but I am OK with the latest version. Will let Viktor weigh in as well. Once this is merged I'll trigger a website update.
Signed-off-by: avifenesh <[email protected]>
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
There are some other formatting changes. I didn't test render it, but I hope you did and checked that they render OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing that would be nice to mention: valkey-doc
exists at least on Fedora and it gives you man pages for all Valkey commands. We could list it as an optional step.
If you want, you could check if this exists on other package managers too...
Signed-off-by: avifenesh <[email protected]>
Ok did the comparison, the indent doesn't change the look, its just md standards. |
Co-authored-by: Viktor Söderqvist <[email protected]> Signed-off-by: Avi Fenesh <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]> Signed-off-by: Avi Fenesh <[email protected]>
Fixes #221
Related PR: valkey-io/valkey-io.github.io#202