Posted by pwolanin on April 3, 2009 at 3:09am
7 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | task |
| Priority: | normal |
| Assigned: | michaelfavia |
| Status: | closed (fixed) |
Issue Summary
follow-up to: #254491: Standardize static caching
see last patch examples: http://drupal.org/node/254491#comment-1430180 and also: http://drupal.org/node/224333#static_variable_api
Apply this conversion pattern to all modules to convert all static variables there. Pay close attention to any place a reset parameter is provided and add a call to drupal_static_reset() where appropriate (e.g. in any calling function that uses the reset parameter)
Note this can be considered a place-holder issue - comment if you are working on a specific module.
Comments
#1
Per discussion w/ catch - if you have a variable name more complex than just __FUNCTION__, use a ':' to separate the suffix (this will avoid colliding with any other valid function name). e.g. :
&drupal_static(__FUNCTION__ . ':second_var');#2
Im starting on node module now and will post patch as i move through them.
#3
First stab at this process please review thoroughly to make sure the rest of these come in cleanly next week. -mf
#423806: Update user module to use drupal_static()
#423822: Update node module to use drupal_static()
#4
Here's some more:
#480102: Update aggregator module to use drupal_static()
#480112: Update block module to use drupal_static()
#480122: Update blogapi module to use drupal_static()
#480412: Update book module to use drupal_static()
#480414: Update comment module to use drupal_static()
#480416: Update filter module to use drupal_static()
#480418: Update forum module to use drupal_static()
#480424: Update locale module to use drupal_static()
#480426: Update menu module to use drupal_static()
#480428: Update openid module to use drupal_static()
#480430: Update search module to use drupal_static()
#5
And some more:
#481498: Update simpletest module to use drupal_static()
#481500: Update syslog module to use drupal_static()
#481502: Update system module to use drupal_static()
#481504: Update translation module to use drupal_static()
#481506: Update trigger module to use drupal_static()
#481508: Update update module to use drupal_static()
#6
This was created a few days ago:
#447862: Convert field to the new static caching API
#7
Most of these sub-issues are pretty quick fixes (except for field, locale, node, simpletest, and user).
#8
There are still two static vars:
modules/simpletest/drupal_web_test_case.php:875: static $available;modules/system/system.queue.inc:63: static $queues;
Both vars in defined within class definitions. I'm assuming you can't register those since there may be multiple instances of the respective classes. Still, it seems like the code is designed so that there won't ever be multiple instances of these two classes. Should these static vars be left alone? Please advice.
#9
The last submitted patch failed testing.
#10
setting this one issue as critical to track that all sub-issues are resolved for D7 and that the conversion gets completed
#11
Rerolled some of the sub-issues.
Locale, search, and filter still need work.
#12
The remaining issues are now locale, search and trigger. IMHO, locale and search are RTBC, while trigger needs only a tiny code-style fix. Once these three are in, this meta-issue can be closed...
#13
Re-rolled #423822: Update node module to use drupal_static()
#14
A search also reveals 39 uses in 19 files of 'static $var' in /modules and 72 in 29 files inside /includes as of today.
Are we trying to clean all of these up?
#15
@agentrickard, some uses of local statics are legitimate and will not be converted. I forgot the exact criteria, but apparently drupal_static should only be used for variables that might conceivably be reset at some point.
#16
Right, I read the docs on API. Have we sanity checked all the remaining statics?
#17
I'm marking this fixed, if we have un-resettable statics, we can find them individually.
#18
Automatically closed -- issue fixed for 2 weeks with no activity.