-
Notifications
You must be signed in to change notification settings - Fork 177
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
[ws client]: parse path
from the URL
#335
Conversation
This commit changes that the path/HTTP resource path is parsed from the URL and removes that option from WsClientBuilder.
/// Sets the URL to pass during the HTTP handshake. | ||
/// | ||
/// The default URL is `/`. | ||
pub fn with_handshake_url(mut self, url: impl Into<Cow<'a, str>>) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unsed, thus removed.
@@ -175,8 +175,7 @@ pub struct WsClientBuilder<'a> { | |||
max_request_body_size: u32, | |||
request_timeout: Option<Duration>, | |||
connection_timeout: Duration, | |||
origin: Option<Cow<'a, str>>, | |||
handshake_url: Cow<'a, str>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed this and parses it from the URL instead
@@ -210,8 +208,8 @@ impl<'a> WsClientBuilder<'a> { | |||
} | |||
|
|||
/// Set request timeout. | |||
pub fn request_timeout(mut self, timeout: Option<Duration>) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
annoying builder method; unrelated to this PR
@@ -210,8 +208,8 @@ impl<'a> WsClientBuilder<'a> { | |||
} | |||
|
|||
/// Set request timeout. | |||
pub fn request_timeout(mut self, timeout: Option<Duration>) -> Self { | |||
self.request_timeout = timeout; | |||
pub fn request_timeout(mut self, timeout: Duration) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
annoying builder method; unrelated to this PR
I tested the fix and that's a PASS for me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good sleuthing here. Left a few suggestions, up to you if you want to implement them. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the change. 👍 LGTM
Closing #306
This commit changes that the path/HTTP resource path is parsed from the URL and removes that option
from WsClientBuilder.