Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Rename FeedsSource->feed_nid to FeedsSource->id and FeedsSource->id to FeedsSource->importer_id.
Alternatively we could rename
FeedsSource->feed_nid to FeedsSource->source_id and keep FeedsSource->id the way it is.
I prefer the former as it 'id' without further specification looks like the id of the object it is found on. Not so the case right now, where FeedsSource->id points to the importer id.
Comments
Comment #1
alex_b CreditAttribution: alex_b commentedForgot to mention: feed_nid should go completely as a source can be attached to anything, not just nodes. (UI does not allow this yet, but I am successfully attaching sources to users).
Comment #2
alex_b CreditAttribution: alex_b commentedComment #3
alex_b CreditAttribution: alex_b commentedThis is postponed to Drupal 7.
Comment #4
zhangtaihao CreditAttribution: zhangtaihao commentedComing from #917678: Refactor code to use arbitrary types of feeds source in addition to nodes.
If I were to give this a shot for D6, would you look at it?
Comment #5
zhangtaihao CreditAttribution: zhangtaihao commentedJust in case your answer is yes, would it be possible to set up a branch point for, say, DRUPAL-6--2?
Comment #6
alex_b CreditAttribution: alex_b commentedThis is a lot of work to get done for D6 and would require a major version move for Feeds. If you're prepared to bite off a large piece of refactoring work and seeing through an upgrade path for it, let's talk.
My gut is telling me that you may get away with a custom solution to the use case you'll need to solve. To be very clear: from an API level it is possible to 'attach' a Feeds source to anything with a serial id. (users, comments, terms, etc.) - there is just no UI for it and you will have to live with the fact that 'feed_nid' is going to reference a user id, comment id, term id.
I'm available on IRC this further.
Comment #7
zhangtaihao CreditAttribution: zhangtaihao commentedI'm in Australia so there's probably quite a slim chance we can find each other on IRC (I just tried).
Would you mind terribly if you import DRUPAL-6--1 onto the github repo as a branch so I can fork it and work from there (or you might include me)?
Comment #8
alex_b CreditAttribution: alex_b commentedGreat call. There you go, just merged in changes from CVS DRUPAL-6--1 right now: http://github.com/lxbarth/Feeds/tree/DRUPAL-6--1
Comment #9
zhangtaihao CreditAttribution: zhangtaihao commentedThanks. I've forked my own DRUPAL-6--2 branch.
Comment #10
alex_b CreditAttribution: alex_b commentedI think it's worth revisiting the naming conventions here.
CTools is using 'name' instead of 'id'. Drupal core uses 'machine_name' - I think that's too long. I like 'name' better than 'id' as it is closer to Ctools, D7 and it suggests that the content is a string and not a serial id.
Further, we should refrain from using 'id' as nowhere in Drupal we find an 'id' key but rather keys like 'nid', 'cid', 'fid', etc.
So here's my proposal:
Comment #11
zhangtaihao CreditAttribution: zhangtaihao commentedYour proposal seems logical. I will roll those changes in.
I should mention that it won't matter too much what we rename anything to, since there would have to be an upgrade path tailored for any renaming anyway.
Also, I've reopened #917678: Refactor code to use arbitrary types of feeds source in addition to nodes so we can focus on the renaming issue here and keep other refactoring matters in that issue.
EDIT:
I'm not sure how the versioning thing works in d.o, but is it possible to create a new version (or branch) for this project here, i.e. 6.x-2.x, without uploading a release? I feel uncomfortable writing about this issue and that as 1.x issues.
Comment #12
zhangtaihao CreditAttribution: zhangtaihao commentedChanging this issue to D7 2.x.
Comment #13
zhangtaihao CreditAttribution: zhangtaihao commentedI'm sorry for spamming this issue. I think I should still give this a try from the D6 end.
Comment #14
zhangtaihao CreditAttribution: zhangtaihao commentedEverywhere I turn I run into the vast amount of functionality tied into the Node API, especially CRUD mechanisms. There is also significant integrated functionality for other modules, e.g. OG, Locale, Taxonomy, etc. It's not worth doing for D6 just yet. I'm now thinking what you did in #3.
However, if I'm not mistaken, 7.x-2.x is still tightly coupled to the Node API (and for that matter depend on nodes). Might there be a chance to refactor it to use the Entity API for CRUD instead? Integrated functionality can at some point be shifted down to Node subclasses of everything. I'm guessing this wouldn't be feasible for 7.x-2.x as that branch is obviously the poster child for D7 (and thus needs to be released as quickly as possible).
Perhaps in 7.x-3.x?
Comment #15
alex_b CreditAttribution: alex_b commentedRenaming according to #10
Comment #16
alex_b CreditAttribution: alex_b commentedThis is better.
Comment #17
alex_b CreditAttribution: alex_b commentedYes, this could be done. One thing I'd like to avoid however, is to use the Field API for attaching a Feeds importer to an entity. Reasons: Field API has a swappable storage back end that introduces all kinds of complications when it comes to accessing data as you can't assume that it's sitting in your SQL database.
This would be all possible in 7.x 2.x as long as it is still an alpha branch.
Comment #18
kenorb CreditAttribution: kenorb commentedClosed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.