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.

Comments

Category:feature» bug

I think this is actually a bug, not a feature request, so refiling.

Yes this is somewhat a bug because it's not Drupal coding rules but PHP rules...

Should we fix it in D5 or D6?

If D6, I'll take a stab at it during DrupalCon, if not before.

Status:Active» Needs review
StatusFileSize
new4.02 KB

First core patch!

Status:Needs review» Needs work

Patch fails in locale.module and is missing a newly introduced cache_set call in theme.inc. Please update.

-K

Status:Needs work» Needs review
StatusFileSize
new4.66 KB

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.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new4.66 KB

Right. The patch is rerolled because patch was complaining a bit about it but not touched in any way.

StatusFileSize
new4.64 KB

Removes unnecessary calls to default variable 'cache'

StatusFileSize
new4.64 KB

There may be an end-of-line problem with patch 28. This one tests correctly.

Status:Reviewed & tested by the community» Fixed

Committed to CVS HEAD. Thanks.

Status:Fixed» Needs work

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?

Status:Needs work» Needs review

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

Status:Needs review» Closed (fixed)

documented