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

feat: improve client instantiation examples #492

Merged
merged 2 commits into from
Mar 9, 2024

Conversation

erdemtuna
Copy link
Contributor

Resolves #435.

  • I restructured the document so that client instantiation is placed at the top. In the old version, the section was at the very bottom, which was inconvenient for a new reader.
  • I also improved the URL build options text by looking at rueidis/url.go

@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.49%. Comparing base (d83782c) to head (c021254).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #492      +/-   ##
==========================================
- Coverage   95.50%   95.49%   -0.01%     
==========================================
  Files          78       78              
  Lines       32941    32941              
==========================================
- Hits        31460    31457       -3     
- Misses       1275     1277       +2     
- Partials      206      207       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rueian
Copy link
Collaborator

rueian commented Mar 9, 2024

Hi @erdemtuna, thank you very much. The content of these two sections looks good to me.

However, I put them at the bottom of the README intentionally because I like to prioritize the command builder and the auto pipelining. I believe the getting started section is already sufficient for new readers and they can find more details later at the bottom.

If we really need to put these initialization sections to the top, I think we had better further shorten their paragraph and merge them into the getting started section, or we can just leave them at the bottom.

@erdemtuna
Copy link
Contributor Author

Hi @rueian, thank you for the feedback. I will try to come up with another alternative based on your concerns.

@erdemtuna
Copy link
Contributor Author

@rueian what do you think about the new version?

@rueian
Copy link
Collaborator

rueian commented Mar 9, 2024

Thanks! That looks good!

@rueian rueian merged commit b08dc62 into redis:main Mar 9, 2024
1 check passed
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.

Add documentation regarding Replica nodes
3 participants