feat(http): Add support for working with Form buffers - #2532
Conversation
github-actions
Bot
added
c-http
|
My previous |
|
Seems like the latest Lavalink PR let a Clippy warning slip through... |
No it was unrelated to any code changes in that PR, I guess it is just a new case caught by default. |
|
Can't request a review but imo this is ready to merge |
| #[allow(clippy::missing_const_for_fn)] | ||
| pub fn form(mut self, form: Form) -> Self { | ||
| /// Set the the multipart form from an existing boundary and buffer. | ||
| pub fn multipart(mut self, boundary: [u8; 15], buffer: Vec<u8>) -> Self { |
There was a problem hiding this comment.
I think we can simplify this. This is essentially a wrapper around the new Form::from_parts method and inlines the new RequestBuilder::form. Because Form is publicly exported from twilight_http::request I think it would be simpler to make Form::from_parts public and have users call the new RequestBuilder::form method. This way we're only increasing the API surface of this type by one method instead of two.
To summarize:
- Remove
multipart - Make
Form::from_partspublic
What do you think?
There was a problem hiding this comment.
The reason I've added this method was for it to match the json/body methods where you have one method which expects a Serializable struct and one which expects raw bytes.
So in case of forms I made it so now have one which expects a Form and one which expects raw bytes.
Asking users to call two methods (Form::from_parts and then RequestBuilder::form) sounds like a worse user experience but I get where you are coming from.
There was a problem hiding this comment.
I would like to hear opinions on this matter from a couple more people.
03c4004 to
7e23bb4
Compare
3 participants
Add support for building
FormbasedRequests using raw form buffers. This is useful when the buffer is transferred through a medium which does not know about theFormstruct (e.g. WASM components).