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.

Sign up for GitHub

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

Jump to bottom

feat(server): Provide reference to Response body #1244

Merged
merged 1 commit into from
Jul 13, 2017
Merged

feat(server): Provide reference to Response body #1244

merged 1 commit into from
Jul 13, 2017

Conversation

bradleybeddoes
Copy link
Contributor

The Response body was previously inaccessible to application code. This
change allows applications that may wish to alter the Response based on
previous creation (or not) of the body to do so.

n.b. discussion in #1216 indicates this function may change in
0.12 or be removed in favour of some other api.

Closes #1216.

@bradleybeddoes
feat(server): Provide reference to Response body
a79fc98 
The Response body was previously inaccessible to application code. This
change allows applications that may wish to alter the Response based on
previous creation (or not) of the body to do so.

n.b. discussion in #1216 indicates this function may change in
0.12 or be removed in favour of some other api.

Closes #1216.
@bradleybeddoes bradleybeddoes mentioned this pull request Jul 6, 2017
Closed
@seanmonstar
Copy link
Member

Hm, I was just about to merge and figure that we'd deal with body types in a 0.12 release, and did a final look over a noticed that, it seems Response at least doesn't actually expose really anywhere that there's an Option inside, right? So, while Request would need new methods, Response could perhaps be fixed...?

@bradleybeddoes
Copy link
Contributor Author

Sure. What did you have in mind, re: fixing Response here?

@seanmonstar
Copy link
Member

Crap, I think the Response::new and Response::default not having a bounds on B means the Option has to stay...

@seanmonstar seanmonstar merged commit 5f47d72 into hyperium:master Jul 13, 2017
@seanmonstar
Copy link
Member

Yea, it can't really be changed in 0.11, so may as well add things like this to allow inspection. In 0.12, we'll have to make some breaking changes so that the the internal Option can be removed, and then an optional body would be up to the user, where Response<Option<Body>> vs Response<Body>.

Sorry for futzing around so long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bradleybeddoes @seanmonstar