The parameter order of cache_set() does not follow Drupal's coding standards. Ref: http://drupal.org/node/318.
Arguments with default values go at the end of the argument list. Always attempt to return a meaningful value from a function if one is appropriate.
http://api.drupal.org/api/5/function/cache_set
cache_set($cid, $table = 'cache', $data, $expire = CACHE_PERMANENT, $headers = NULL)
Specifically, since $data is not default, it should occur before $table.
cache_set($cid, $data, $table = 'cache', $expire = CACHE_PERMANENT, $headers = NULL)
Writing the function the second way also allows porting of 4.x.x modules to 5.x and higher without rewriting all cache functions.
Comment | File | Size | Author |
---|---|---|---|
#9 | cache_28_0.patch | 4.64 KB | agentrickard |
#8 | cache_28.patch | 4.64 KB | agentrickard |
#7 | cache_27.patch | 4.66 KB | chx |
#6 | cache_26.patch | 4.66 KB | agentrickard |
#4 | cache_set_3.patch | 4.02 KB | agentrickard |
Comments
Comment #1
agentrickardI think this is actually a bug, not a feature request, so refiling.
Comment #2
chx CreditAttribution: chx commentedYes this is somewhat a bug because it's not Drupal coding rules but PHP rules...
Comment #3
agentrickardShould we fix it in D5 or D6?
If D6, I'll take a stab at it during DrupalCon, if not before.
Comment #4
agentrickardFirst core patch!
Comment #5
Zen CreditAttribution: Zen commentedPatch fails in locale.module and is missing a newly introduced cache_set call in theme.inc. Please update.
-K
Comment #6
agentrickardWell, when it takes 3 weeks to review a core patch to a primary function, sometimes core changes.
This needs to get in ASAP, before any more changes are made to the cache system.
Comment #7
chx CreditAttribution: chx commentedRight. The patch is rerolled because patch was complaining a bit about it but not touched in any way.
Comment #8
agentrickardRemoves unnecessary calls to default variable 'cache'
Comment #9
agentrickardThere may be an end-of-line problem with patch 28. This one tests correctly.
Comment #10
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #11
Dries CreditAttribution: Dries commentedI'm marking this 'code needs work' until the upgrade instructions in the handbook have been properly updated. Can you take a look at that agentrickard?
Comment #12
agentrickardI don't have edit permissions for the parent (http://drupal.org/node/114774), so I made a child page.
http://drupal.org/node/136533
Comment #13
agentrickarddocumented