Replace async_trait with native async fns#230
Conversation
|
@djc we have an issue with the coverage pipeline: |
djc
left a comment
There was a problem hiding this comment.
This mostly looks good to me. Please squash into two commits:
- MSRV bump
- All the code changes
| /// error, it will be forwarded to the configured error sink. | ||
| async fn on_acquire(&self, _connection: &mut C) -> Result<(), E> { | ||
| Ok(()) | ||
| fn on_acquire<'a, 'b, 'r>(&'a self, _connection: &'b mut C) -> BoxFuture<'r, Result<(), E>> |
There was a problem hiding this comment.
Can we spell this <'r, 'a: 'r, 'b: 'r> instead? And do we really need the 3 separate lifetimes?
Why is BoxFuture useful? Could we just do Box<dyn Future<..>> instead?
There was a problem hiding this comment.
BoxFuture<'a, T> is actually Pin<Box<dyn Future<Output = T> + Send + 'a>>. I'll apply the suggestions.
There was a problem hiding this comment.
And do we really need the 3 separate lifetimes?
I'm not sure, but that's what async_trait generated, so I kept it to prevent further breaking changes in the API.
|
I think #231 has solved the coverage issue, please rebase on top of current main. |
|
@djc I applied the following suggested changes:
Let me know if you have any further suggestions. |
|
Please also add a separate commit that bumps all the version numbers. |
|
@djc done. |
|
Thanks! |
|
Note that the usage of
CustomizeConnectionrequires it to be object safe, and thus we need to use boxed futures instead of plain async fns.