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.
Problem/Motivation
The purpose of hook_library() and hook_library_alter() is not clear from their names.
Proposed resolution
Per sun: Rename these functions to follow existing patterns:
hook_library()
should be renamed tohook_library_info()
.hook_library_alter()
should be renamed tohook_library_info_alter()
.
Remaining tasks
TBD
User interface changes
None.
API changes
hook_library()
will be renamed tohook_library_info()
.hook_library_alter()
will be renamed tohook_library_info_alter()
.
Comment | File | Size | Author |
---|---|---|---|
#9 | rename-hook-library-1286868-5023772.patch | 8.18 KB | rickmanelius |
#6 | rename-hook-library-1286868-5020994.patch | 7.71 KB | rickmanelius |
#3 | rename-hook-library-1286868-5020458.patch | 5.24 KB | rickmanelius |
Comments
Comment #1
sunThanks! :)
Comment #2
xjmComment #3
rickmanelius CreditAttribution: rickmanelius commentedpatch attached using search replace for both functions
Comment #4
xjmWe need to actually change the name of the hook where it gets called as well (e.g. in drupal_get_library()). Look for
module_invoke()
anddrupal_alter()
with'library'
as an argumentComment #5
rickmanelius CreditAttribution: rickmanelius commentedxjm. Apologies for that oversite. Will fix and submit another patch.
Comment #6
rickmanelius CreditAttribution: rickmanelius commentedTry this one. I also realized I missed a few implemented hook_library hooks. I manually set them after the larger search/replace changes.
I only found the instances of module_invoke and drupal_alter instances mentioned above. I also checked invoke_all and didn't find anything. So unless there is something else, this should be set.
Comment #7
sun#6 is RTBC after someone else than @rickmanelius applied the patch and verified/double-checked/grepped that all instances are covered indeed.
Comment #8
xjmLooks like we still missed a few references to module implementations. In
common.test
, there is a missed reference tocommon_test_library()
, and insystem.api.php
, there is a missed reference tosystem_library()
. (I just grepped for_library(
with the patch applied and skipped lines withdrupal_add_library()
anddrupal_get_library()
.)Might want to go through all instances of
library
to be sure we have everything.Comment #9
rickmanelius CreditAttribution: rickmanelius commentedThanks again xjm for your patience and your feedback. I did a search for both _library and then library to make sure. I found the ones you described and I think there were 1-2 others. Let's try this one more time.
Comment #10
rickmanelius CreditAttribution: rickmanelius commentedComment #11
xjmI read the patch closely and checked all instances of
library
with the patch applied. Assuming testbot comes back green, I think this is RTBC.Comment #12
tstoecklerCan't believe I almost missed that one. :)
Cool stuff, patch looks awesome!
Comment #13
rickmanelius CreditAttribution: rickmanelius commentedAwesome. Thanks for all the help/instructions, xjm.
Comment #14
Dries CreditAttribution: Dries commentedGood clean-up. Committed to 8.x. Thanks.
Comment #15
jhodgdonThis patch is marked "fixed", and "api change", but there is no change node. Needs one I think?
Comment #16
rickmanelius CreditAttribution: rickmanelius commentedHi jhodgdon.
New contributor here. How would one do that? Can you provide a link/example? I can address if it's within my skill set.
Comment #17
jhodgdonSee
http://drupal.org/node/1203498
(there's a section on change notifications in the Docs section at the top) (and ask again if that didn't answer your question)
Comment #18
webchickComment #19
gddhttp://drupal.org/node/1294416
Comment #20
webchickLooks good. Thanks!
Restoring previous status.
Comment #22
xjmComment #22.0
xjmUpdated issue summary.