Book module is shown in the example, but it no longer implements hook_init().
dblog.module does, to add CSS, but i suspect it's not following best practice.
Should dblog instead be using '#attached' to add CSS to its page's render array?
And if adding CSS and JS no longer a recommended use for hook_init(), is there another example out there? locale or overlay module perhaps?
Comment | File | Size | Author |
---|---|---|---|
#12 | system-hookinithookboot-1008250-12.patch | 2.72 KB | bxtaylor |
#2 | 1008250.patch | 2.67 KB | jhodgdon |
Comments
Comment #1
jhodgdonLet's keep this issue as related to documentation only. File a separate issue to ask about what dblog should be doing.
I agree, the example in hook_init() is bad.
http://api.drupal.org/api/drupal/modules--system--system.api.php/functio...
The CSS is now being added by the book.info file I think.
The doc for hook_init() has some other problems too, which we should fix, such as that the first line should be one sentence, and hook_boot() should have () after it to make it into a link. hook_boot() is also lame in the same way, and both have grammar issues.
It appears that there are several example implementations of hook_init() and hook_boot() on api.drupal.org, so we should be able to come up with better examples.
Comment #2
jhodgdonIt looks like someone changed the example in hook_init() already. How are all of these documentation issues getting fixed without getting into the documentation queue??? I looked at 3 issues just now that had all been fixed. Mysterious.
Anyway, there were still some cleanup issues for these two hooks, so here is a patch.
Comment #3
jhodgdon#2: 1008250.patch queued for re-testing.
Comment #4
mlncn CreditAttribution: mlncn commentedThe cleanup is good and the patch applies fine!
Comment #6
jhodgdonThat failure is bogus. Doc-only patch causing fatal error?
Comment #7
jhodgdon#2: 1008250.patch queued for re-testing.
Comment #8
jhodgdonAt this point, this is d8 material. Hopefully that will launch a retest.
Comment #9
jhodgdon#2: 1008250.patch queued for re-testing.
Comment #10
xjmWent to open an issue for this, and hey, it exists. :)
This needs to be "That's" or (better) "That is".
Other than that, the patch will need to be rerolled in the new format. This looks like a reroll a novice could handle.
Comment #11
xjmComment #12
bxtaylor CreditAttribution: bxtaylor commentedRerolled: changed Thats to That is and changed non cached to non-cached. Here's the patch.
Comment #13
jhodgdonLooks good to me! Should be d7/8. Thanks!
Comment #14
Dries CreditAttribution: Dries commentedCommitted to 7.x and 8.x. Thanks.