-
Notifications
You must be signed in to change notification settings - Fork 4
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
chore: where possible replace async-trait
with native async trait support in 1.75+
#197
Conversation
Performance Benchmark Report
Code Coverage Report: Only Changed Files listed
Minimum allowed coverage is |
Thanks for looking at this. Do you know if there's a way to keep the simpler return types (using the 'async' keyword's syntactic sugar)? At least for the simpler cases. Also, for recursive cases, would be interesting to know if there are any crates with macros that are 1.75+ but "do the boxing for us" under the hood. This would also be mainly to keep the code slightly simpler to read/maintain. |
That's what |
That sounds like a better direction. Let's remove unnecessary |
4dfa2d6
to
55dc085
Compare
a0a3f7e
to
94202e3
Compare
94202e3
to
cca73ce
Compare
@reubeno done |
d36894f
to
2fe6ba7
Compare
2fe6ba7
to
17e09dc
Compare
LGTM. Thanks also for addressing some warnings along the way. I'll get this merged, but if you find additional warnings like these that aren't being caught by the GitHub PR workflows, then it would be interesting to figure out how we could tweak things to catch them in the future. |
async-trait
async-trait
with native async trait support in 1.75+
…upport in 1.75+ (reubeno#197) Reduce usage of the async-trait crate where possible, using the async_fn_in_trait native feature in 1.75+.
…upport in 1.75+ (reubeno#197) Reduce usage of the async-trait crate where possible, using the async_fn_in_trait native feature in 1.75+.
As of Rust 1.75, the
async_fn_in_trait
feature has been merged (see: rust-lang/rust#115822), meaning the async-trait crate is now only required for dynamic dispatch (dyn Trait). Since brush doesn’t usedyn Command
, I decided to try removing the dependency.The modulesarithmetic.rs
andinterp.rs
are most affected, as they use async recursion, which requires futures to be boxed. I’ve wrapped all recursive functions inasync move { .. }.boxed()
.