-
Notifications
You must be signed in to change notification settings - Fork 7
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
[WIP] Common feed format #5
base: master
Are you sure you want to change the base?
[WIP] Common feed format #5
Conversation
Do we want to expose only the common format as the public API? |
Thanks for the contribution! We should only expose the common format as the public API. The purpose of this library is to define a common format so that you can use the same code for working on both Atom and RSS feed, but I just never got around to finishing it. Exposing the Atom/RSS library-specific formats defeats that purpose. I'll try to take a look at the code sometime this week. |
Cargo.toml
Outdated
@@ -1,13 +1,14 @@ | |||
[package] | |||
name = "syndication" | |||
version = "0.1.1" | |||
authors = ["Tom Shen <[email protected]>"] | |||
version = "0.1.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should rebase your branch, since the library is already on version "0.2.0." This change should bump the version to "0.3.0," since it breaks the original API.
Use the Entry conversions in the Atom Feed conversions
62ca37d
to
1012a5b
Compare
Right now I'm considering doing use atom_syndication as atom; since it gives better symmetry with |
That looks fine to me. |
The public API shouldn't expose any types from rss or atom_syndication, so this adds all the necessary types.
Add Guid type since the RSS Guid is more detailed than the Atom one.
Add the Image and TextInput types to support the change.
EntryData and FeedData need an explicitly implementation of Debug because the atom types don't support it, and we probably don't want to show the contained feeds anyway.
5c1498e
to
c11d7ef
Compare
There aren't very good tests for this library, are there. Trying to write some, I guess I can see why. Any ideas for what I should do on the testing front? |
Should I put myself as an author? |
You could use tests from other open source libraries. e.g. https://github.com/kurtmckee/feedparser/tree/develop/tests
Definitely, especially since after this PR merges, most of the codebase will be your work. Honestly, I haven't looked at Rust stuff in a really long time. So if you want, I can just transfer ownership of the repo/crate to you. |
I just finished up classes for the year, and I accidentally broke my main RSS reader, so I figure now's a good time to come back to this. I'm going to try to use this to build a small feed reader in order to figure out if the API has any glaring holes. |
Hi, is there still motivation for this? I'm currently writing an application that requires unified RSS & Atom parsing, this would be a great help. Willing to pick it up if there's no time for this on your side. |
My job means I'm currently not able to easily make open source contributions, I would be happy with someone else picking up the work. |
This is a first attempt at defining a common structure for both Atom and RSS feeds. It's not complete yet, and it doesn't provide the best round trip guarantees right now. Feedback is very much appreciated.
In addition to looking at the docs for both atom_syndication and rss, the information at RSS and Atom Compared has been extremely helpful while implementing this.