Misc. small changes #57
Conversation
…loses #53) * move request method to `RestClient` trait for better separation of concerns * move execute method to `Execute` trait for better type inference
| type Error; | ||
|
|
||
| fn request<T, U>( |
There was a problem hiding this comment.
You are breaking the abi with this modification.
There was a problem hiding this comment.
Yes I'd like to propose this breaking change to API because IMHO request is better placed in the REST interface, as it's a shorthand for the implementation of the Method specific methods
| pub consumer_key: String, | ||
| pub consumer_secret: String, | ||
| #[derive(Clone, PartialEq, Eq)] | ||
| pub struct Signer<T = String> { |
There was a problem hiding this comment.
What is the idea behind adding a generic type which is defaulting to the current type? To me, this is over-engineered, please keep it simple stupid
There was a problem hiding this comment.
I think it actually makes things easier and also prevents many intermediate allocations, since we most handle static string slices and types with internal string slice representations (Method, Url). I pushed things further in #58 by removing the trait altogether and letting the implementation always work on string slices directly which drastically simplifies it all, IMHO
| urlencoding::encode(&self.consumer_secret), | ||
| urlencoding::encode(&self.secret) | ||
| urlencoding::encode(self.consumer_secret.as_ref()), | ||
| urlencoding::encode(self.secret.as_ref()) |
There was a problem hiding this comment.
What is the difference between the two version? To me, it is easier to read with the previous version
There was a problem hiding this comment.
With String with have ops::Deref<Target = str> but with the proposed change we are generic on T: AsRef<str> which is a broader and must use the method explicitly
SseErrorOftype aliasreqwest::IntoUrl(closes Allow endpoint to be any type that implementsIntoUrl#53)RestClienttrait for IMHO better separation of concernsExecutetrait for better type inferencefmt::Debugimpl