Posted by mlncn on December 27, 2010 at 5:58pm
6 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | documentation |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | needs backport to D7, Novice |
Issue Summary
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?
Comments
#1
Let'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.
#2
It 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.
#3
#2: 1008250.patch queued for re-testing.
#4
The cleanup is good and the patch applies fine!
#5
The last submitted patch, 1008250.patch, failed testing.
#6
That failure is bogus. Doc-only patch causing fatal error?
#7
#2: 1008250.patch queued for re-testing.
#8
At this point, this is d8 material. Hopefully that will launch a retest.
#9
#2: 1008250.patch queued for re-testing.
#10
Went to open an issue for this, and hey, it exists. :)
+++ modules/system/system.api.php 12 Jan 2011 05:47:27 -0000@@ -672,7 +672,7 @@
+ * hook_init() instead. Thats the usual case. If you implement this hook
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.
#11
#12
Rerolled: changed Thats to That is and changed non cached to non-cached. Here's the patch.
#13
Looks good to me! Should be d7/8. Thanks!
#14
Committed to 7.x and 8.x. Thanks.
#15
Automatically closed -- issue fixed for 2 weeks with no activity.