Skip to content

Conversation

@andy-stark-redis
Copy link
Contributor

@andy-stark-redis andy-stark-redis commented Mar 28, 2025

DOC-4996

Note: there is more to add to the hiredis docs, but I thought I'd get some feedback on this basic set. The C code is all tested but I'd be more than happy to hear suggestions for good/idiomatic C practice.

Copy link
Collaborator

@dwdougherty dwdougherty left a comment

Choose a reason for hiding this comment

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

Very nice, @andy-stark-redis! Apart from one non-blocking comment, LGTM.

Comment on lines +13 to +14
linkTitle: hiredis (C)
title: hiredis guide (C)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not clear to me how hiredis is to be capitalized. The GitHub page uses "Hiredis"; but some blogs use either "HIREDIS" or "hiredis". It's not a showstopper by any means; just me being overly pedantic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, tbh, I just picked one and tried to stick with it :-) I'll be happy to change it if anyone has a definitive version of the name.

@andy-stark-redis andy-stark-redis requested a review from uglide March 28, 2025 14:28
@andy-stark-redis
Copy link
Contributor Author

Thanks for the review @dwdougherty !

@andy-stark-redis andy-stark-redis merged commit 7857d00 into main Mar 28, 2025
5 checks passed
@andy-stark-redis andy-stark-redis deleted the DOC-4996-hiredis-docs branch March 28, 2025 16:06
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.

4 participants