GNOME Bugzilla – Bug 785454
Add support for RSS enclosures
Last modified: 2017-07-28 11:46:13 UTC
Some RSS feed have enclosures (e.g. podcasts) and such enclosures are supported by the ontology. The RSS miner should extract these elements and link them to the messages.
Created attachment 356436 [details] [review] Add support for feed enclosures This is a first implementation of the enclosure support for RSS miner. It uses the `mfo:Enclosure` and `nfo:RemoteDataObject` classes to store the information and the `mfo:enclosureList` predicate to link messages to its enclosures.
Created attachment 356437 [details] [review] Add support for feed enclosures New patch without tab expansion.
Review of attachment 356437 [details] [review]: Looks mostly good! thanks for the patch :). Some comments follow: ::: src/miners/rss/tracker-miner-rss.c @@ +715,3 @@ +static void +sparql_add_enclosure (TrackerSparqlBuilder *sparql, + gchar *subject, The subject argument is not modified here, better make it const gchar* @@ +886,3 @@ + + // subject is not freed right now as it will be freed at the end of the + // method in a foreach loop over the contrib_aliases list Tracker uses /* */ comments in C code, and it's not properly indented :). Also, ugh... were we doubly freeing the strings? Probably worth a separate bug (or at least patch) on its own. @@ +891,3 @@ + + i = 0; + for(l = enclosures; l; l = l->next) { Missing space between for and ( @@ +902,3 @@ + + // subject is not freed right now as it will be freed at the end of the + // method in a foreach loop over the enclosure_aliases list Again, /* */ comments, please :) @@ +1002,2 @@ g_list_free (contrib_aliases); + g_list_free (enclosure_aliases); Now that you're at it, please use g_list_free_full(), also for contrib_aliases.
Created attachment 356443 [details] [review] Add support for feed enclosures Hi Carlos, Thanks for the really quick review. Sorry for my code style issues. Here is a new version of the patch with your remarks taken into account.
Comment on attachment 356443 [details] [review] Add support for feed enclosures Thanks! The patch looks good for master. If you don't have a GNOME git account I'll eventually push it.
Created attachment 356444 [details] [review] Add support for feed enclosures New patch with two enhancements: - enclosures are deleted together with their referencing message - avoid adding twice enclosures with same URL in a message
(In reply to Carlos Garnacho from comment #5) > Comment on attachment 356443 [details] [review] [review] > Add support for feed enclosures > > Thanks! The patch looks good for master. If you don't have a GNOME git > account I'll eventually push it. I have no GNOME git account. Thanks for your feedback.
Pushed to the master branch. Thanks for your contribution!