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

Suggestion: rename Value to CommandResult #7

Open
rw opened this issue Dec 10, 2019 · 3 comments
Open

Suggestion: rename Value to CommandResult #7

rw opened this issue Dec 10, 2019 · 3 comments

Comments

@rw
Copy link

rw commented Dec 10, 2019

I almost wrote an issue asking for fallible Value types, but then I realized that Value is meant to handle errors, too. It would be clearer to new users to call that type something like CommandResult.

@Bunogi
Copy link
Owner

Bunogi commented Dec 11, 2019

The idea is that Value represents any valid Redis value, excluding errors. This was inspired by crates like toml and serde_json. When an error message is encountered, deserialization fails and returns the error message as an Error::RedisError(...). It seemed to be the approach which made the most sense from an API point of view, but it's a tradeoff.

As such, I'm not sure if renaming it to that is a good idea, considering returning Result<Value> is clearer than Result<CommandResult>. I can see returning a simple CommandResult, but I'm concerned about the fallout for error handling in general, as it would allow Ok(CommandResult::RedisErr(...)).

@rw
Copy link
Author

rw commented Dec 14, 2019

The idea is that Value represents any valid Redis value, excluding errors.

How are errors handled when getting results of pipelined operations?

@Bunogi
Copy link
Owner

Bunogi commented Dec 15, 2019

Currently, it will return an error if any of the operations fail. It should probably be changed to returning a stream, which would allow the user to pick their preferred behaviour.

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

No branches or pull requests

2 participants