The attached patch attempts to add simple georss support for common syndication parser. Simple as defined by: http://georss.org/Main_Page and points and placenames only. A related patch to the data module for testing purposes is forthcoming.

Comments

cglusky’s picture

@rsoden I think this is the Data patch you mentioned. #836878: Support mapping of complex terms

cglusky’s picture

i suppose this gets into modularity of the architecture and what gets into the core module and what needs to be in it's own module etc. i think georss info in a feed is common enough these days to rate being in Common Syndication Parser.

@rsoden i am guessing if you want this to go anywhere you will need to provide simple tests to go along with the changes - asserting that this can grab points and placeneames from a feed etc. @alex_b can say for sure. and i wish i could help but i have not figured out simple tests for my own patches.

r,
c

alex_b’s picture

Status: Needs review » Needs work

This is a great start.

- I don't see where FeedsGeoLocation is being used.
- Tests would indeed be great here. Could be as simple as adding GeoRSS information and mapping into FeedsRSStoNodesTest.
- There's stray white space.
- There are unrelated modifications to FeedsDataProcessor

boris mann’s picture

Awesome. If this is similar enough to "common", then roll it in.

I would say that from a UI perspective, having a GeoRSS mapper might provide some more UI hooks to integrate into, and be more understandable for end users.

cglusky’s picture

Title: Add simple georss supoort to common syndication parser » Add simple georss support to common syndication parser

fixing typo in title to help search index

Will White’s picture

Status: Needs work » Needs review
StatusFileSize
new4.72 KB

Updated patch to address some of the issues mentioned above.

- Removed FeedsGeoLocation class
- Removed stray white space.
- Removed unrelated modifications to FeedsDataProcessor

alex_b’s picture

Status: Needs review » Needs work

This is looking good. Minor stuff:

1. $lat and $lon don't seem to be reset after each iteration.
2. Code style needs to be fixed: $lat.' '.$lon

Will White’s picture

Status: Needs work » Needs review
StatusFileSize
new4.46 KB

This patch now resets those variables and fixed the style.

alex_b’s picture

Issue tags: +6.x-1.0-beta5

To be released w/ 6.x-1.0-beta5.

alex_b’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new26.91 KB

Fixed an issue where GUID was dropped, added tests. Will commit asap.

alex_b’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)
Issue tags: -6.x-1.0-beta5

Automatically closed -- issue fixed for 2 weeks with no activity.