The block API uses a set of magic variables (mainly 0, 1 and 2) that aren't defined anywhere. This makes the code less readable than it could be and makes the API itself less clean than it could be.
The block.install file referred to cache defines that no longer exist in the module, I've updated that. It also used hardcoded integers where it should have been using the DRUPAL_CACHE_* defines from common.inc
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 769540-3.patch | 10.07 KB | cafuego |
| #4 | 769540-2.patch | 10.19 KB | cafuego |
| #1 | 769540.patch | 9.68 KB | cafuego |
Comments
Comment #1
cafuego commentedAttached patch updates block.install to use provided defines and adds some defines for a cleaner block API.
Comment #2
cafuego commentedComment #3
casey commentedLooks good. You need to document constants separately however (have a look at node.module).
Also, I am not sure if this should be considered an API change (which then should go into D8).
Comment #4
cafuego commentedWell, since the defined values are the same and the integers still work I'd like to think of it as a cleanup instead of an API change ;-)
3rd party modules will still work fine even if they don't use the defines - though this might encourage module developers to tidy up their code too.
Updated patch with per-define documentation (shamelessly copied from admin/block) attached.
Comment #6
cafuego commentedFine, so I stuffed up a copy & paste. Fixed the patch, reattached, re-queued for testing.
Comment #7
casey commentedCode is good. I don't see this as an API change either. Let's see what Dries/webchick thinks.
Comment #8
dries commentedCommitted to CVS HEAD. Thanks.