Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency on failure #82

Merged
merged 2 commits into from
Jan 11, 2020
Merged

Conversation

SSheldon
Copy link
Contributor

@SSheldon SSheldon commented Dec 29, 2019

This used to be a dependency of quick-xml, but it was removed in version 0.17 by tafia/quick-xml#170. rss depended on it previously to work with quick-xml::Error, but this is no longer necessary. Keeping it means rss is pulling in a lot of other unnecessary dependencies.

This change also updates to stop using the deprecated parts of std's Error trait.

@SSheldon
Copy link
Contributor Author

Hmm, I just noticed that the sibling atom_syndication crate accepted a PR to replace failure with thiserror in rust-syndication/atom#17.

This handwritten code for the error is really not much, and it doesn't seem worth pulling in more dependencies to avoid it to me...

@SSheldon
Copy link
Contributor Author

SSheldon commented Jan 6, 2020

I took the liberty of including an updated from the deprecated Error APIs to bring this in-line with atom after the changes in rust-syndication/atom#18.

@SSheldon
Copy link
Contributor Author

SSheldon commented Jan 9, 2020

Now that rust-syndication/atom#18 has been merged, this change will bring the design of the errors in rss and atom in line.

Anything else I need to do before this can be merged?

@frewsxcv
Copy link
Member

Anything else I need to do before this can be merged?

LGTM!

@DCjanus DCjanus merged commit 33cc5bf into rust-syndication:master Jan 11, 2020
@SSheldon SSheldon deleted the rm_failure branch January 11, 2020 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants