search

rust-syndication / rss

Library for serializing the RSS web content syndication format
https://crates.io/crates/rss
Apache License 2.0
419 stars 52 forks source link

Cleanup following #35 #38

Closed jameshurst closed 7 years ago

frewsxcv commented 7 years ago

Minus my comment, this all looks good to me!

Regarding the from_url method on Channel: Personally, if I had to pick between having that functionality available out-of-the-box or available as opt-in feature (like in this PR). I definitely don't feel strongly about it though, there are pros and cons to either option. And if it turns out people really want that functionality enabled in the future by default, we can bring it up again.

@PhnxRbrn Have any thoughts about these changes?

chrisppy commented 7 years ago

@frewsxcv Any rss reader would use the from_url method, so to me it does not make much sense behind a feature gate

jameshurst commented 7 years ago

@PhnxRbrn My thought is that an rss application may want to use their own networking library or customize the request used to retrieve the rss feed. If a user doesn't want to use this feature then we're including a pretty large dependency (hyper) for no reason.

chrisppy commented 7 years ago

@jameshurst very true, with that in mind, it does make sense to keep it behind the flag.

frewsxcv commented 7 years ago

thanks again everyone, going to merge and publish this :)