Reorder cache_set() parameters
agentrickard - February 13, 2007 - 15:03
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | other |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
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.
#1
I think this is actually a bug, not a feature request, so refiling.
#2
Yes this is somewhat a bug because it's not Drupal coding rules but PHP rules...
#3
Should we fix it in D5 or D6?
If D6, I'll take a stab at it during DrupalCon, if not before.
#4
First core patch!
#5
Patch fails in locale.module and is missing a newly introduced cache_set call in theme.inc. Please update.
-K
#6
Well, 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.
#7
Right. The patch is rerolled because patch was complaining a bit about it but not touched in any way.
#8
Removes unnecessary calls to default variable 'cache'
#9
There may be an end-of-line problem with patch 28. This one tests correctly.
#10
Committed to CVS HEAD. Thanks.
#11
I'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?
#12
I don't have edit permissions for the parent (http://drupal.org/node/114774), so I made a child page.
http://drupal.org/node/136533
#13
documented