refactor!(client): replace the optional RequestConfig arg to Client::send() with a with_request_config() builder method
#4443
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4443 +/- ##
==========================================
+ Coverage 85.42% 85.44% +0.02%
==========================================
Files 283 283
Lines 31556 31566 +10
==========================================
+ Hits 26958 26973 +15
+ Misses 4598 4593 -5 ☔ View full report in Codecov by Sentry. |
…t::send()` with a `with_request_config()` builder method
Instead of `Client::send(request, request_config)`, consumers can now do
`Client::send(request).with_request_config(request_config)`. The
parameter is still `Option`al to avoid messy patterns, when the
`request_config` is an `Option` passed from the caller.
The messy pattern would look like this, if `with_request_config` took a
`RequestConfig` and not an `Option<RequestConfig>`:
```
let mut send_req = client.send(req);
if let Some(request_config) = request_config {
send_req = send_req.with_request_config(request_config);
}
```
bnjbvr
force-pushed
the
bnjbvr/remove-config-from-clientsend
branch
from
8260578 to
c7b87f3
Compare
| @@ -77,6 +77,13 @@ impl<R> SendRequest<R> { | |||
| self | |||
| } | |||
|
|
|||
| /// Use the given [`RequestConfig`] for this send request, instead of the | |||
| /// one provided by default. | |||
| pub fn with_request_config(mut self, request_config: Option<RequestConfig>) -> Self { | |||
There was a problem hiding this comment.
Why would we take an option here? Not calling this is the same as passing None, no?
There was a problem hiding this comment.
It's to avoid the messy pattern described in OP and commit message. I was a bit torn about this.
There was a problem hiding this comment.
Could do impl Into<Option<RequestConfig>> so people can pass a RequestConfig directly. There's no reason to ever call it with None (as in, write that out in code such that the T of Option::<T>::None can't be inferred) so type inference should not be an issue.
Instead of
Client::send(request, request_config), consumers can now doClient::send(request).with_request_config(request_config). The parameter is stillOptional to avoid messy patterns, when therequest_configis anOptionpassed from the caller.The messy pattern would look like this, if
with_request_configtook aRequestConfigand not anOption<RequestConfig>: