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

expose Headers::new and Extensions::new #320

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jbr
Copy link
Member

@jbr jbr commented Jan 16, 2021

Other users than http-rs might want to use these types, but they cannot currently be constructed outside of this crate

@yoshuawuyts yoshuawuyts mentioned this pull request Jan 18, 2021
@Fishrock123
Copy link
Member

needs a rebase for clippy

@jbr jbr force-pushed the expose-headers-new-and-extensions-new branch from 3a8faa5 to d7ec64a Compare January 21, 2021 01:36
@yoshuawuyts
Copy link
Member

I'm okay with this change, but think we should document that this is most likely not what you want to do. Someone coming from hyperium/http may incorrectly assume that Headers instances need to manually be created. Instead this really only serves as an intermediate type that is meant to be accessed through Request, Response, etc. Much like a trait is, but slightly different.

Just exposing this without guidance may cause confusion, and we should try and evade that if possible. (Though it'd be correct to point out that our current guidance on these types isn't great either, and folks may already be confused. But ideally we shouldn't add to that 😅 )

@yoshuawuyts

This comment has been minimized.

@Fishrock123
Copy link
Member

I think this could be useful if you could set the headers when constructing a request, at least that would make per-client headers in Surf config a tad nicer under the hood.

@jbr jbr mentioned this pull request Jul 12, 2021
@CfirTsabari
Copy link

I think this could be useful if you could set the headers when constructing a request, at least that would make per-client headers in Surf config a tad nicer under the hood.

why not using Middleware?

@Fishrock123
Copy link
Member

Unimportant implementation detail, but feature wise, convenience.

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.

4 participants