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

Make Script#invoke_async generic over aio::ConnectionLike #242

Merged
merged 1 commit into from
Oct 9, 2019

Conversation

emschwartz
Copy link
Contributor

Instead of specifying a SharedConnection. This enables wrapping the SharedConnection with another type that implements the appropriate traits.

I needed this in order to build a connection wrapper (also discussed in #218) that implements reconnection logic.

Instead of specifying a SharedConnection. This enables wrapping the SharedConnection with another type that implements the appropriate traits.
@emschwartz
Copy link
Contributor Author

@Marwes @badboy @mitsuhiko any feedback on this?

@Marwes Marwes merged commit e71d16a into redis-rs:master Oct 9, 2019
@Marwes
Copy link
Collaborator

Marwes commented Oct 9, 2019

LGTM Thanks!

This is a breaking change (though a small one) so it needs a breaking release.

@emschwartz
Copy link
Contributor Author

Thanks!

What makes it breaking? SharedConnection implements aio::ConnectionLike so all existing calls to it should keep working, no?

@Marwes
Copy link
Collaborator

Marwes commented Oct 9, 2019

Adding a type parameter to a function cause turbo fish to no longer work query::<T> //Error

@emschwartz
Copy link
Contributor Author

@Marwes Any chance you could publish a new version with this change soon? (I understand if you want to wait to get other PRs in before bumping the version but would love to update my code to use this feature this week if possible)

@badboy
Copy link
Collaborator

badboy commented Oct 14, 2019

@emschwartz Release in progress!

@badboy
Copy link
Collaborator

badboy commented Oct 14, 2019

oh damn, didn't see the breaking, I'll retro-actively mention that in the changelog though.

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