Open tgeoghegan opened 4 months ago
In general, I'm trying to avoid pulling in a whole lot of dependencies, which the http
-related changes would. An adapter (or From/Into implementations) would be OK with a feature gate.
As for ohttp
vs. bhttp
, the two are completely independent from a code perspective. The convenience functions you are talking about adding might end that, so we can maybe wrap one error in the other for those functions.
I don't have a ton of spare time for doing this work, but I'd be happy to help out with reviews and whatnot.
What if this was http::StatusCode instead? Or perhaps there's a good reason that bhttp/ohttp avoid depending on crate http.
While avoiding a dependency on http
is noble and admirable, I can't think of any environment where bhttp
/ohttp
would be used without a dependency on http
somewhere in the tree.
Our use of this crate in Firefox is an example of exactly that, as it happens. Pulling in http
would mean a much bigger binary. I'm OK with these conveniences being present, but they would need to be optional, I think.
I want to note a grab bag of minor usability nits to see whether the maintainer(s) care about them and/or agree with my suggestions. These would involve changing or adding to public API so there are meaningful maintainability considerations here.
bhttp
andohttp
is a chore. Is anyone ever going to useohttp
withoutbhttp
? In particular, this is annoying because I wind up having to define separate enum variants in my error type forohttp::Error
andbhttp::Error
, as does glean/Viaduct.bhttp
has its ownbhttp::StatusCode
which is an alias foru16
. What if this washttp::StatusCode
instead? Or perhaps there's a good reason thatbhttp
/ohttp
avoid depending on cratehttp
.bhttp::Message
to things likehttp::Response
orhttp::Request
. These would be easy to gate behind a feature if there's a concern about dragginghttp
into everyone'sCargo.lock
.ClientResponse
that does theCursor
dance for you and spits out a [Message
] (or, in the context of #66, a more specific response type).