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

Changes for (Add IsParseError() documentation #578) #581

Merged
merged 5 commits into from
Jun 28, 2024

Conversation

karimalzalek
Copy link
Contributor

Removed the section mentioning panic as behaviour is changed, and added a short error handling paragraph to explain errParse and IsPareseErr helper.

Copy link
Contributor

@SoulPancake SoulPancake left a comment

Choose a reason for hiding this comment

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

LG

README.md Outdated

```golang
// Attempt to parse the response. If a parsing error occurs, check if the error is a parse error and handle it.
if err := client.Do(ctx, client.B().Get().Key("k").Build()).AsInt64(); IsParseErr(err) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if err := client.Do(ctx, client.B().Get().Key("k").Build()).AsInt64(); IsParseErr(err) {
if err := client.Do(ctx, client.B().Get().Key("k").Build()).ToArray(); IsParseErr(err) {

Hi @karimalzalek, thank you for fixing the document but your example, GET+AsInt64, will not return an errParse. Maybe using something like ToArray as an example will be less confusing.

Also, could you help us emphasize that a user should normally fix the code by choosing the correct parser function, not using IsParseErr, in the comment?

@karimalzalek
Copy link
Contributor Author

Hello @rueian , I changed the example to ToArray(), and emphasized that the user needs to specify the correct parser beforehand. Let me know if the changes make sense.

@rueian
Copy link
Collaborator

rueian commented Jun 25, 2024

Hi @karimalzalek, the PR looks good to me but please remove the .vscode/settings.json.

Removing wrong settings.json file
@karimalzalek
Copy link
Contributor Author

@rueian yup, I removed the wrong config file, not sure how that made it's way through 😂

Co-authored-by: Anurag Bandyopadhyay <angbpy@gmail.com>
@rueian rueian merged commit b19f550 into redis:main Jun 28, 2024
19 checks passed
@rueian
Copy link
Collaborator

rueian commented Jun 28, 2024

Thanks @karimalzalek!

SoulPancake added a commit to SoulPancake/rueidis that referenced this pull request Jun 29, 2024
Changes for (Add IsParseError() documentation redis#578) (redis#581)
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.

3 participants