Initial stab at converting to use Serde 1.0

[andrew-d]

This is an initial attempt to convert the library to use Serde 1.0 (ref: #108). It's the first time I've done this particular conversion, so there's a couple things that aren't yet finished. In particular, there's some uses of #[serde(bound(deserialize = ""))] which I'm not happy about; that particular solution comes from https://github.com/serde-rs/serde/issues/891.

Any advice appreciated - or, if other folks want to build off this work, happy to have anyone use this as a starting point.

[andrew-d]

(also: it currently compiles, but there's test failures; will debug later this week)

[andrew-d]

Bleh, that was a silly bug. Okay, test all pass, though I'm still not happy with the deserialize = "" bound. Perhaps some folks more experienced with serde can chime in? @KodrAus perhaps, since you filed the original issue?

[benashford]

Thanks for this :+1:

As you say it's probably worth taking a bit of time to look into some of these edge-cases, to make sure we're happy with the direction. I'm quite busy today so I probably won't be able to have a proper look until later this week. But in the meantime if anyone else has any comments, etc., the more the merrier.

Thanks again.

[KodrAus]

Thanks @andrew-d! You're right that #[serde(bound(deserialize = ""))] is a smell. The best way to avoid needing it is to remove the DeserializeOwned bound from the structs that have it, then you can remove that bound. For example GetResult would become:

/// The result of a GET request
#[derive(Debug, Deserialize)]
pub struct GetResult<T> {
    #[serde(rename="_index")]
    pub index:    String,
    #[serde(rename="_type")]
    pub doc_type: String,
    #[serde(rename="_id")]
    pub id:       String,
    #[serde(rename="_version")]
    pub version:  Option<u64>,
    pub found:    bool,
    #[serde(rename="_source")]
    pub source:   Option<T>
}

serde will take care of ensuring T: Deserialize<'de> behind the scenes.

Are you ok with removing those bounds @benashford? I don't think they're super valuable on struct definitions, because you're going to bound that T later anyways.

[andrew-d]

I removed the bounds and the tests have all passed; if this is okay with @benashford, we can merge as-is, or otherwise happy to find some other option 😄

[benashford]

This looks good, thanks @andrew-d and thanks @KodrAus for the review :+1:

I'll merge this and release it as version 0.10.0 due to the updated dependencies shortly.

[benashford]

Now published as 0.10.0

[andrew-d]

Thanks for the quick turnaround, folks 😀