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

Go: Add command zlexcount #3140

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Go: Add command zlexcount #3140

wants to merge 2 commits into from

Conversation

tjzhang-BQ
Copy link
Collaborator

@tjzhang-BQ tjzhang-BQ commented Feb 12, 2025

Issue link

This Pull Request is linked to issue (URL): #2972

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Create merge commit if merging release branch into main, squash otherwise.

@tjzhang-BQ tjzhang-BQ added the go golang wrapper label Feb 12, 2025
@tjzhang-BQ tjzhang-BQ requested a review from a team as a code owner February 12, 2025 06:02
@@ -526,3 +526,23 @@ func ExampleGlideClient_ZUnionWithScores() {
// Output:
// map[one:1 three:3 two:5.5]
}

func ExampleGlideClient_ZLexCount() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also add an example for ExampleGlideClusterClient_ZLexCount()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added, although is it necessary that we make separate examples for single key commands like this one? it's same code with the same results

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. Examples in Go are keyed to documentation by the func name. So ExampleGlideClient_ZLexCount will show in the documentation for the GlideClient type. The ExampleGlideClusterClient_ZLexCount will show in the documentation for the GlideClusterClient type. This ensures that regardless of which environment you are in you will see the correct documentation, with examples. You and I know the examples are the same, but an end user does not know that, and there are a couple instances where that is not the case (e.g. CustomCommand)

@@ -526,3 +526,23 @@ func ExampleGlideClient_ZUnionWithScores() {
// Output:
// map[one:1 three:3 two:5.5]
}

func ExampleGlideClient_ZLexCount() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. Examples in Go are keyed to documentation by the func name. So ExampleGlideClient_ZLexCount will show in the documentation for the GlideClient type. The ExampleGlideClusterClient_ZLexCount will show in the documentation for the GlideClusterClient type. This ensures that regardless of which environment you are in you will see the correct documentation, with examples. You and I know the examples are the same, but an end user does not know that, and there are a couple instances where that is not the case (e.g. CustomCommand)

@@ -526,3 +526,43 @@ func ExampleGlideClient_ZUnionWithScores() {
// Output:
// map[one:1 three:3 two:5.5]
}

func ExampleGlideClient_ZLexCount() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider adding multiple examples. The integration tests offer a couple examples with simple setup that can be easily refactored into examples. The goal is to make it easy on the developer so they don't have to download source code and dig around in our test files just to figure out how to use the client.

Copy link
Collaborator

@yipin-chen yipin-chen left a comment

Choose a reason for hiding this comment

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

LGTM. Please address comments and resolve conflicts.

TJ Zhang added 2 commits February 18, 2025 16:45
Signed-off-by: TJ Zhang <tj.zhang@improving.com>
Signed-off-by: TJ Zhang <tj.zhang@improving.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go golang wrapper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants