search

michaelnisi / pickup

Transform XML feeds
MIT License
21 stars 1 forks source link

Added lastBuildDate mappings to updated, to support wordpress RSS feeds #14

Closed naxxfish closed 5 years ago

naxxfish commented 5 years ago

It appears that in the wonderful world of RSS inconsistencies, some feeds do not have pubDate in their <channel>s. But they do have lastBuildDate, which is near as damn-it the same thing.

Here's a feed which does this: https://answermethispodcast.com/feed/

naxxfish commented 5 years ago

I should mention, the failed build is from an error in one of the dev dependencies, tap

michaelnisi commented 5 years ago

Thanks @naxxfish. Falling back on lastBuildDate seems useful. Please add a test to prove that pubDate takes precedence when both exist (to meet existing assumptions).

naxxfish commented 5 years ago

Good shout, and interestingly it doesn't. How does this work in the entry map where there is a mapping for both content and summary - is it expected that entries should never have both of these elements inside them?

naxxfish commented 5 years ago

Figured it out. Had to slightly refactor a bit of the ontext function being used with sax so that it made more sense (namely renaming the function shouldOverride to shouldSummaryOverride, and subsequently creating shouldUpdatedOverride in a similar style). Also had to nest some more ifs to ensure that it didn't break the existing summary override code, which meant the ifs within the shouldSummaryOverride function became unnecessary.

Hopefully there won't be too many more of these sorts of edge cases, as it's significantly less elegant than the rest of the module!

michaelnisi commented 5 years ago

Thanks for doing this, @naxxfish. I’m going to merge after contemplating edge cases vs elegance. That makes two edge cases from the same bucket, there are probably more. Maybe we could use the order of the mappings to model priority.

Happy 2019 🎉

michaelnisi commented 5 years ago

Thanks 🙌🏼 d3f411c4b1170cb37cc65d72aa496fe7fb400db6

michaelnisi commented 5 years ago

I’ve tried the thought I had the other day, using the mappings key order to define precedence. Performance hasn’t been considered yet, but shouldn’t be hard to optimize if required at all.

https://github.com/michaelnisi/pickup/pull/15

@naxxfish What do you think?