Change send_json data argument

[k0nserv]

Hello,

Request::send_json has the following signature:

    pub fn send_json(mut self, data: impl serde::Serialize) -> Result<Response>;

However, the implementation only ever uses references to data making the API less flexible than it could be. For example, I'm implementing some logic that needs to perform retries and because ownership is passed I need to clone my data, despite the function not needing ownership of it.

This would be a breaking change, although one that is trivially fixable, so it would need to be slated for 3.0. I'd also suggest reviewing all API surfaces for similar redundant passing of ownership and relaxing to references where appropriate.

Thanks!

[algesten]

Yeah I agree, this could be passed by reference. Can you make a bullet in FUTURE.md?

[k0nserv]

Can do!

[k0nserv]

Here's a PR #739

[algesten]

Danke

[bebecue]

if T impls serde::Serialize, so does &T.

so fn send_json(data: impl serde::Serialize); can accept data passed by ownership or reference.

I'm implementing some logic that needs to perform retries and because ownership is passed I need to clone my data, despite the function not needing ownership of it

in this case, we can pass a reference to send_json() instead.

fn main() {
    send_json(NotClone {
        name: "foo".to_string(),
    });

    let retries = 3;
    let data = NotClone {
        name: "foo".to_string(),
    };
    for _ in 0..retries {
        send_json(&data);
    }
}

#[derive(serde::Serialize)]
struct NotClone {
    name: String,
}

fn send_json(data: impl serde::Serialize) {}

if we change it to fn send_json(data: &impl serde::Serialize);, then it can only accept data passed by reference.

so I think we can keep the old function signature and be flexible.

[k0nserv]

if T impls serde::Serialize, so does &T.

Only if serde provides a blanket implementation for &T, but it seems it does. Things is I tried a &T before I raised this issue, but I'll double check since you should be correct @bebecue

[k0nserv]

I retested @bebecue and you are right, I don't know how I ended up breaking this. My bad. Reverted the note in FUTURE.md in #811