search

nmdias / FeedKit

An RSS, Atom and JSON Feed parser written in Swift
MIT License
1.19k stars 174 forks source link

Add iTunes Podcasting Tags Namespace #10

Closed bamurph closed 7 years ago

bamurph commented 7 years ago

I've been using FK for a podcast client I'm working on, and decided to go ahead and create a namespace for Apple's iTunes Podcasting tags. Due to the dominance of the iTunes directory it's become a de facto standard for podcasting.

Even though the standard RSS2 tags have everything necessary for a valid & usable podcast feed, its very likely any particular feed will make use of the iTunes tags in addition to the RSS2 tags.

This is my pull-request / nontrivial contribution to a project on GitHub, so please let me know if I've got anything out of order or that needs fixed. I've tried to stick with your code formatting idioms as much as possible and have (I think) a reasonable level of test coverage.

Thanks!

Ben

nmdias commented 7 years ago

I do have some suggestions.

Could you move the iTunesPodcasting.xml into the existing xml folder?

Also, the iTunesCategory.swift, iTunesNamespace.swift and iTunesOwner.swift I was thinking maybe we could place those in an iTunes folder inside the Modules folder?

Other than that everything looks great. Travis is failing, but I'm guessing a restart will fix it.

Outstanding contribution!

Thanks

bamurph commented 7 years ago

Oh sure thing, yeah I forgot to check the actual folder structure, I hadjust put them in groups in Xcode. I'll update now!

Ben

Sent from my iPhone

On Jan 30, 2017, at 5:38 PM, Nuno Dias notifications@github.com wrote:

I do have some suggestions.

Could you move the iTunesPodcasting.xml into the existing xml folder?

Also, the iTunesCategory.swift, iTunesNamespace.swift and iTunesOwner.swift I was thinking maybe we could place those in an iTunes folder inside the Modules folder?

Other than that everything looks great. Travis is failing, but I'm guessing a restart will fix it.

Outstanding contribution!

Thanks

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

bamurph commented 7 years ago

I did have to fix target memberships on the "toDuration" string extension to get it to pass, but after that it looks like the issue remaining is this 'exit 65' https://github.com/travis-ci/travis-ci/issues/6675 - It seems like they've come up with a workaround that warms up the simulator ahead of running the tests, but tbh I've never used Travis before so I'm hesitant to muck around with it at the moment.

I did restart my own failed TI job and it passed though so I assume that'll work here.