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

RFC: My implementation of a body for Request #5

Merged
merged 8 commits into from
Nov 27, 2022

Conversation

StratusFearMe21
Copy link

@StratusFearMe21 StratusFearMe21 commented Nov 22, 2022

I went ahead and made the PR of the API I mentioned in #3, So far it works great and is very performant, but I'd still like to know what y'all think about my implementation.

@StratusFearMe21
Copy link
Author

I've just added this comment on line 52 of request.rs under the Body's resolve() function

/// This is preferable over using `std::io::Read` if your `Body` is small.

I'm pretty sure this is correct, but I'm not 100% sure.

@StratusFearMe21
Copy link
Author

I've just improved the safety of Request using lifetime magic, however, it is a breaking change (but considering this project is unreleased that probably doesn't matter)

@Xudong-Huang
Copy link
Owner

I like the impl, but could we first make a feature guard to avoid affect the existing project?

@Xudong-Huang Xudong-Huang changed the base branch from master to dev November 27, 2022 11:01
@Xudong-Huang Xudong-Huang merged commit 9417866 into Xudong-Huang:dev Nov 27, 2022
@Xudong-Huang
Copy link
Owner

we can first merge the this into dev branch, we would merge it back to mater when matured

@RoadtogreatWorld
Copy link

Can this already be merged into master? I need to use it and dev branch is so behind.

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