Replace error-returning builders with typestate-based builders #330
Comments
seanmonstar
added
the
B-rfc
|
Agreed. When looking into why Take the |
Fixes hyperium#330. Creating a Response with the builder pattern should not require handling errors unless opting in by using a `try_`-method (which returns a `Result<Builder2>`, not a `Builder2` with an interal error state). This is really a breaking change to `response::Builder`, but rather than suggstig a breaking change directly, I introduce an alternative `response::Builder2`. If this is liked, the original `Builder` can be deprecated, and in a later release `Builder2` can be renamed back to `Builder`.
|
@seanmonstar , how does the |
|
The tag is meant to say the issue needs more comments about how and if to move forward. The discussion can happen on the issue. |
|
Ok. I have added some thoughts. Maybe I should add some more motivation / explanation on the linked PR. |
Fixes one part of hyperium#330. This makes it possible to create an `Uri` using the builder pattern without hanlding errors. This is really a breaking change to `uri::Builder`, but rather than suggesting a breaking change directly, I introduce an alternative `uri::Builder2`. If this is accepted, the original Builder is deprecated, and in a later release Builder2 can be renamed back to Builder. The new `Builder2` is generic on its contents, which starts out as the empty type `()`. The parts `Scheme`, `Authority` and `PathAndQuery` can be added in any combination, and the `build` method is implemented for valid combinations only.
Fixes one part of hyperium#330. Creating a `Request` with the builder pattern should not require handling errors unless opting in by using a `try_`-method (which returns a `Result<Builder2>`, not a `Builder2` with an internal error state). This is really a breaking change to `request::Builder`, but rather than suggesting a breaking change directly, I introduce an alternative `response::Builder2`. If this is accepted, the original `response::Builder` is deprecated, and in a later release `Builder2` can be renamed back to `Builder`.
Fixes one part of hyperium#330. Creating a `Request` with the builder pattern should not require handling errors unless opting in by using a `try_`-method (which returns a `Result<Builder2>`, not a `Builder2` with an internal error state). This is really a breaking change to `request::Builder`, but rather than suggesting a breaking change directly, I introduce an alternative `response::Builder2`. If this is accepted, the original `response::Builder` is deprecated, and in a later release `Builder2` can be renamed back to `Builder`.
Fixes hyperium#330. Creating a Response with the builder pattern should not require handling errors unless opting in by using a `try_`-method (which returns a `Result<Builder2>`, not a `Builder2` with an interal error state). This is really a breaking change to `response::Builder`, but rather than suggstig a breaking change directly, I introduce an alternative `response::Builder2`. If this is liked, the original `Builder` can be deprecated, and in a later release `Builder2` can be renamed back to `Builder`.
Fixes one part of hyperium#330. This makes it possible to create an `Uri` using the builder pattern without hanlding errors. This is really a breaking change to `uri::Builder`, but rather than suggesting a breaking change directly, I introduce an alternative `uri::Builder2`. If this is accepted, the original Builder is deprecated, and in a later release Builder2 can be renamed back to Builder. The new `Builder2` is generic on its contents, which starts out as the empty type `()`. The parts `Scheme`, `Authority` and `PathAndQuery` can be added in any combination, and the `build` method is implemented for valid combinations only.
Fixes one part of hyperium#330. Creating a `Request` with the builder pattern should not require handling errors unless opting in by using a `try_`-method (which returns a `Result<Builder2>`, not a `Builder2` with an internal error state). This is really a breaking change to `request::Builder`, but rather than suggesting a breaking change directly, I introduce an alternative `response::Builder2`. If this is accepted, the original `response::Builder` is deprecated, and in a later release `Builder2` can be renamed back to `Builder`.
|
Aim for fewer open merge requests. |
Fixes hyperium#330. Creating a Response with the builder pattern should not require handling errors unless opting in by using a `try_`-method (which returns a `Result<Builder2>`, not a `Builder2` with an interal error state). This is really a breaking change to `response::Builder`, but rather than suggstig a breaking change directly, I introduce an alternative `response::Builder2`. If this is liked, the original `Builder` can be deprecated, and in a later release `Builder2` can be renamed back to `Builder`.
Fixes one part of hyperium#330. This makes it possible to create an `Uri` using the builder pattern without hanlding errors. This is really a breaking change to `uri::Builder`, but rather than suggesting a breaking change directly, I introduce an alternative `uri::Builder2`. If this is accepted, the original Builder is deprecated, and in a later release Builder2 can be renamed back to Builder. The new `Builder2` is generic on its contents, which starts out as the empty type `()`. The parts `Scheme`, `Authority` and `PathAndQuery` can be added in any combination, and the `build` method is implemented for valid combinations only.
Fixes one part of hyperium#330. Creating a `Request` with the builder pattern should not require handling errors unless opting in by using a `try_`-method (which returns a `Result<Builder2>`, not a `Builder2` with an internal error state). This is really a breaking change to `request::Builder`, but rather than suggesting a breaking change directly, I introduce an alternative `response::Builder2`. If this is accepted, the original `response::Builder` is deprecated, and in a later release `Builder2` can be renamed back to `Builder`.
Fixes one part of hyperium#330. This makes it possible to create an `Uri` using the builder pattern without hanlding errors. This is really a breaking change to `uri::Builder`, but rather than suggesting a breaking change directly, I introduce an alternative `uri::Builder2`. If this is accepted, the original Builder is deprecated, and in a later release Builder2 can be renamed back to Builder. The new `Builder2` is generic on its contents, which starts out as the empty type `()`. The parts `Scheme`, `Authority` and `PathAndQuery` can be added in any combination, and the `build` method is implemented for valid combinations only.
Fixes one part of hyperium#330. Creating a `Request` with the builder pattern should not require handling errors unless opting in by using a `try_`-method (which returns a `Result<Builder2>`, not a `Builder2` with an internal error state). This is really a breaking change to `request::Builder`, but rather than suggesting a breaking change directly, I introduce an alternative `response::Builder2`. If this is accepted, the original `response::Builder` is deprecated, and in a later release `Builder2` can be renamed back to `Builder`.
Ok, #483 replaces the previous PRs. |
|
There's still not much discussion here. Maybe that's because there isn't that much to discuss? Is there any actual downsides to replacing the current run-time checks in the builder patterns with compile-time typestate-based checks? This is a breaking API change (even if #483 is a non-breaking first step), so it should be taken with caution. But this is a breaking API change, so we should really want it to happen as soon as possible, way before releasing a version 1.0 of the crate. So, for the issue itself, I'm pretty sure there is nothing that should stop this. For the implementation suggested in #483 , there may or may not be problems. Please review! :-) |
|
I would like to see this change. I was surprised to see |
It'd be really handy to replace the current builders with a set of builders for Request, Response, and URI that track the builder's state using some sort of typestate, allowing for compile-time checks that a Request/Response/URI were correctly built.
(Following up from Twitter: https://twitter.com/seanmonstar/status/1149772057212669952)
The text was updated successfully, but these errors were encountered: