Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
rdf_entity_info_alter() contains an old @todo and references to an UI which is irrelevant for core.
Comment | File | Size | Author |
---|---|---|---|
#4 | 1092352-rdf_entity_info_alter-doc_fix-4.patch | 983 bytes | marcin.wosinek |
#2 | 1092352-rdf-docs.patch | 977 bytes | TR |
#1 | 1092352_1_rdf_docs.patch | 969 bytes | scor |
Comments
Comment #1
scor CreditAttribution: scor commentedComment #2
TR CreditAttribution: TR commentedNeeds to go into D8 first.
Reclassifying as a Documentation issue.
Re-rolled patch for the current D8, in -p1 format.
Comment #3
jhodgdonThis looks like a reasonable change... A few suggestions:
a) Can you provide a link to some information on the RDF CRUD mapping API, or at least a function or two that are part of it?
b) "which" always needs a comma before it.
c) table names should be enclosed in {}.
d) Should this new text be a new paragraph? If so, leave a blank line. If not, you might need to move the word "Modules" up to the previous line to get all lines in each paragraph wrapped as close to 80 characters as possible.
Thanks!
Comment #4
marcin.wosinek CreditAttribution: marcin.wosinek commentedImplementation of #3 feedback.
Comment #5
jhodgdonBetter! The link to @see rdf seems like a good way to provide a link to the RDF API, but I'm not sure which functions would be called to do CRUD really... Maybe saying "use whatever_the_function() instead" would be clearer?
I'm also wondering about the word "willing" here... Do you mean "trying"?
Also, this is not a hook that modules can implement... it seems to be a function that modules can call, right?
Comment #6
scor CreditAttribution: scor commentedThe function to use to do CRUD is http://api.drupal.org/api/drupal/modules!rdf!rdf.module/function/rdf_mapping_save/7
rdf_entity_info_alter() is an implementation of hook_entity_info_alter(), which is the hook in question. Does the following make more sense?
Comment #7
jhodgdonTo me, that doesn't make sense. Other modules that want to alter *entity info* would implement hook_entity_info_alter(). That hook is not anything to do with RDF per se, so why would anyone think to implement that hook in order to alter RDF mappings?
Instead, I think if we just remove the @todo section from the function doc here, we'll have the documentation clearly saying "This is a hook implementation", and it would be clear enough that no one should really call this function.
And I guess it wouldn't hurt to reword where it says "Adds the proper RDF mapping to each entity type/bundle pair." ... instead of saying "proper", it could explain where it gets the RDF mappings from (presumably from mappings that have been stored with the RDF save function, right?).
Comment #8
scor CreditAttribution: scor commentedthat would work for me @jhodgdon.
Comment #9
jhodgdonCool, let's get a patch then. :)
Comment #10
sunHm. I actually think it would be good to add the originally suggested comment from the last patch, perhaps phrased a little better.
It seems like the intention was to clarify that other modules should not implement hook_module_implements_alter() in order to have their hook_entity_info_alter() implementation run after RDF module's, so they're able to alter the definitions being injected by RDF module.
Such a comment makes sense to me, because honestly, I think I would have done just that (if I had any need for doing so). ;)
So if that was the intended message, then I think it could be simplified into:
"Other modules should not attempt to alter the mappings being injected here. Use hook_rdf_mapping_blahablah instead."
Comment #19
TR CreditAttribution: TR commented... and back to D7, because this code no longer exists in D8 - it was removed by #1869600: Refactor RDF mappings to be inline with the new Entity Field API
And, to reflect reality, this should be "won't fix" after more than 10 years ...