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

Add support to casting RedisResult to CString. #660

Merged
merged 1 commit into from Aug 18, 2022

Conversation

nihohit
Copy link
Contributor

@nihohit nihohit commented Aug 14, 2022

No description provided.

@nihohit
Copy link
Contributor Author

nihohit commented Aug 14, 2022

The linter seems to fail on code that isn't a part of this PR.

@djc
Copy link
Contributor

djc commented Aug 15, 2022

#663 cleans up the unrelated clippy failures.

@djc djc requested a review from jaymell August 15, 2022 08:30
@djc
Copy link
Contributor

djc commented Aug 15, 2022

This seems reasonable to me. @jaymell what do you think?

redis/src/types.rs Outdated Show resolved Hide resolved
@nihohit nihohit force-pushed the cstring-support branch 2 times, most recently from 13ad495 to 36a641a Compare August 16, 2022 09:58
@jaymell
Copy link
Contributor

jaymell commented Aug 16, 2022

Looks good. Should we add a couple simple tests?

@nihohit
Copy link
Contributor Author

nihohit commented Aug 17, 2022

@jaymell I'll be happy to add some, but I don't see equivalent String tests to copy from.
What tests would you like me to add?

@jaymell
Copy link
Contributor

jaymell commented Aug 18, 2022

@jaymell I'll be happy to add some, but I don't see equivalent String tests to copy from. What tests would you like me to add?

Nothing special, there are some similar examples if you look at the other type tests, e.g., make sure that a null byte in a value returns the error we expect.

@nihohit nihohit force-pushed the cstring-support branch 2 times, most recently from 043435e to 16794e3 Compare August 18, 2022 08:49
@nihohit
Copy link
Contributor Author

nihohit commented Aug 18, 2022

@jaymell added tests.

@djc djc merged commit 7b67951 into redis-rs:main Aug 18, 2022
@djc
Copy link
Contributor

djc commented Aug 18, 2022

Thanks!

@nihohit nihohit deleted the cstring-support branch August 22, 2022 08:53
shachlanAmazon added a commit to shachlanAmazon/glide-for-redis that referenced this pull request Sep 1, 2022
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.

None yet

3 participants