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

Redefine StatusCode in Reqwest #122

Closed
Tracked by #120
seanmonstar opened this issue May 31, 2017 · 11 comments
Closed
Tracked by #120

Redefine StatusCode in Reqwest #122

seanmonstar opened this issue May 31, 2017 · 11 comments
Labels
B-upstream Blocked: upstream. Depends on a dependency to make a change first.
Milestone

Comments

@seanmonstar
Copy link
Owner

From hyperium/hyper#1196, hyper still has a way to go to become stable, and after discussing trying to stabilize a smaller crate of types, it seems like the best action is to define the type in Reqwest, and in the future use the types from some http crate. This allows us to move Reqwest along, without trying to rush stabilizing an http crate without exploration.

@seanmonstar seanmonstar added this to the 1.0 milestone May 31, 2017
@jimmycuadra
Copy link

This would be unfortunate for my use case as I described in hyperium/hyper#894. I am hoping for types that represent the building blocks of HTTP to exist in a crate without a full client implementation.

I'm also not sure I understand how StatusCode could exist in reqwest if it's built on hyper. Does that mean hyper has its own identical StatusCode type?

@seanmonstar
Copy link
Owner Author

We all want an http crate. The point here is that the libs team would like to stabilize reqwest to 1.0 sooner than we could probably stabilize a proper http. So, this is the only solution.

For your second question, it's exactly as it sounds. hyper would of course have a StatusCode, but be unstable. Reqwest would never expose it, only showing its own.

@seanmonstar
Copy link
Owner Author

Blocked on Rust 1.20, as we want associated constants instead of enum variants.

@mathstuf
Copy link
Contributor

mathstuf commented Jul 7, 2017

Wouldn't this also mean that reqwest would be stuck with its own even after a hypothetical http crate materialized (though From and Into make it easy, it's still not ideal)?

Though I wouldn't be against a 2.0 at that point either. It would theoretically be source compatible, but everything would still need to agree on the same major version number.

@seanmonstar
Copy link
Owner Author

If the types don't change at all during the http stabilization, then it should hopefully be a backwards compatible change to just change to a re-export.

@mathstuf
Copy link
Contributor

mathstuf commented Jul 7, 2017

I guess Cargo would have issues bridging the types even if two minor versions of reqwest found each other from different crate transitive dependencies anyways, so there's no difference then between a minor or major bump.

@seanmonstar seanmonstar added the B-upstream Blocked: upstream. Depends on a dependency to make a change first. label Aug 19, 2017
@opilar
Copy link

opilar commented Sep 22, 2017

Rust 1.20 is stable now. I would like to help. What should be done?

@seanmonstar
Copy link
Owner Author

I received some comments from a few on the rust libs team that reqwest should just use the types from the http crate, even though it won't be 1.0 before reqwest. I'm undecided if that's the right thing to do, but welcome other comments.

@KodrAus
Copy link
Contributor

KodrAus commented Sep 22, 2017

Right now reqwest re-exports the status codes from hyper, right? So does this depend on a new version of hyper that uses http? Otherwise I guess you'd have to convert status codes to and from u16 or something between hyper and reqwest.

@seanmonstar
Copy link
Owner Author

No, this wouldn't require a new version of hyper. Eventually hyper will use http types, but before that, one can just convert to hyper's types internally.

@seanmonstar seanmonstar modified the milestones: 1.0, 0.9 Jul 4, 2018
@seanmonstar
Copy link
Owner Author

There is now the http crate, and 0.9 will depend on it directly, so this isn't needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-upstream Blocked: upstream. Depends on a dependency to make a change first.
Projects
None yet
Development

No branches or pull requests

5 participants