Implement serde::Serialize and serde::Deserialize for rss::Item

[huytd]

First, I have to say that this is a great crate! Thank you so much for your hard work!

Most of the case, I ended up returning a list of rss::Items (as a vector or something) and I want to use it in my JSON data, for example:

fn fetch_from(url: &str) -> FetchResult<Vec<Item>> {
    Ok(Channel::from_url(url)?.items().to_vec())
}

fn main() {
    let Ok(result) = fetch_from("https://some-url.com/rss.xml");
    println!("{:?}", json!({ "result": result }));
}

The problem here is rss::Item does not implements serde::Serialize, so I got this error:

error[E0277]: the trait bound `rss::Item: serde::Serialize` is not satisfied
  --> src/main.rs:11:22
   |
11 |     println!("{:?}", json!({ "result": result }));
   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `serde::Serialize` is not implemented for `rss::Item`
   |

So I have to remote implement serde::Serialize fot rss::Item on my own.

I know the crate should be generic and adding a dependencies to some other crate is not a good idea, but since serde is kind of language's standard now. Should we consider implement serde::Serialize directly into this crate?

[frewsxcv]

I'm -1 personally, but I don't feel strongly. In my opinion serializing RSS to JSON is not a common enough usecase to warrant the extra dependency. I also don't want to give people the impression that they can use https://github.com/serde-rs/xml to output valid RSS XML. What do you think @jameshurst ? Another option would be to make it an opt-in optional dependency.

[frewsxcv]

Opened a pull request with opt-in serialization: https://github.com/rust-syndication/rss/pull/55

Let me know how it looks. You can use it by specifying features = ["serialization"] in your cargo.toml

[jameshurst]

I'd agree with @frewsxcv here and say that this wouldn't really be a common enough use case and would probably add confusion regarding have serde output xml. I'd be -1 as well for this.

[djc]

So, the Rust API guidelines say that all types "that play the role of a data structure" should derive these. That seems a fairly strong recommendation. If the rust-syndication crate want to counter this advice, it would be good to start a discussion as part of the API guidelines repo to have some accepted exceptions transcribed in the guidelines.

I'm not sure the argument brought forward here ("we already serialize as XML natively") is very strong. The serde traits can cover anything from JSON to bincode, and there many reasons using one of those might be more attractive than XML for some transport mechanisms or a quick local cache (as we're doing in planetrs -- https://github.com/Vagdish/planetrs/issues/19). In addition, the cost here seems very low, especially as a feature that's disabled by default (ideally enabled with the serde flag, as recommended in the guidelines).

[frewsxcv]

@djc after letting this settle in my head for some months, and after reading your last comment here, i think you're absolutely right. i just updated the serialization pull request to just use the serde flag name. i'll flag you for a review if you've got a minute

[frewsxcv]

merged in https://github.com/rust-syndication/rss/pull/55